feat(agent): close knowledge loop — refuse complete_task without writeback (D.1+D.2)

D.1 — complete_task structural gate:
- hadDiscovery(ctx, session) reports whether the session ran `run` successfully
  against a live target (NOT get_entity/list_lxcs — those are DB lookups, not
  new facts). A trivial Q&A that only calls get_entity is a degenerate case
  and must NOT be blocked.
- complete_task with outcome=success is REFUSED when hadDiscovery && !
  hadEntityWriteback. The refusal fires BEFORE completeTask runs, so the
  session stays in 'executing' state and the agent must call
  update_entity_attributes/create_relationship then retry complete_task.
  An explicit failure/partial is allowed through (the agent is acknowledging
  it didn't finish — no reason to force writeback).
- Replaces the prior advisory warning (5.5) which the agent consistently
  ignored. The agent saw the warning and ended the task anyway; this gate
  makes the writeback a hard prerequisite for success.

D.2 — propose_plan auto-append writeback step:
- When the agent proposes a plan whose steps don't mention
  update_entity_attributes or create_relationship, D.2 appends a final
  'Write back: update_entity_attributes + create_relationship +
  upsert_knowledge' step before persisting. The result string tells the
  agent it was appended.
- With the seq-order enforcement (5.6) and D.1's complete_task gate, the
  agent must complete the writeback step (and actually call the tools) to
  finish. Neither relies on the agent reading SOUL.md.
- Removed the old advisory writeback nudge from propose_plan's result
  string — D.2 makes it structural.
- Updated the propose_plan tool description to state both gates crisply.

Verification:
- TestHadDiscoveryAndWriteback: hadDiscovery true only after a successful
  `run`; false after failed run, get_entity, or no calls. hadEntityWriteback
  true only after update_entity_attributes/create_relationship.
- e2e against the live agent (oikos-nomos-1, v0.5.1):
  - D.2: agent proposed 3 steps (no writeback); D.2 auto-appended step 4
    'Write back: update_entity_attributes + ...'. Result string said
    '(appended a writeback step — your plan didn't include one; step 4)'.
  - D.1: agent ran `run` (uptime on lxc:gitea), called complete_task, was
    REFUSED ('Refused: this session ran run against live targets (discovery)
    but did not call update_entity_attributes...'). Agent self-corrected:
    called update_entity_attributes, retried complete_task, succeeded.
    Knowledge loop closed end-to-end.

Version 0.5.0 -> 0.5.1 (patch: structural enforcement of existing intent).

VERSION

@@ -1 +1 @@
-0.5.0
+0.5.1

cmd/nomos/store.go

@@ -726,6 +726,31 @@ func (s *store) hadEntityWriteback(ctx context.Context, sessionID string) bool {
 	return count > 0
 }
 
+// hadDiscovery checks whether this session ran `run` successfully against a
+// real target — i.e. discovered live state (versions, package counts, host
+// facts, service status) that the DB didn't have. Used by complete_task to
+// refuse success when discovery happened but no writeback followed (the
+// knowledge-loop drift the prior warnings failed to close — the agent
+// ignored advisory text, so D.1 makes it structural).
+//
+// Only `run` counts as discovery here, NOT get_entity/list_lxcs/etc. — those
+// are DB lookups, not new facts. A trivial Q&A ("status of lxc:dns?") that
+// only calls get_entity is a degenerate case (SOUL.md: "Don't invent
+// attributes that don't exist") and must NOT be blocked. Only sessions that
+// actually executed against a live target get the writeback gate.
+func (s *store) hadDiscovery(ctx context.Context, sessionID string) bool {
+	if s == nil || sessionID == "" {
+		return false // fail safe: don't block when we can't check
+	}
+	var count int
+	s.pool.QueryRow(ctx, `
+		SELECT COUNT(*) FROM agent_activity
+		WHERE session_id = $1
+		  AND tool_name = 'run'
+		  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).

cmd/nomos/store_test.go

@@ -18,6 +18,7 @@ import (
 	"testing"
 
 	"github.com/dtoro/oikos/internal/db"
+	"github.com/google/uuid"
 	"github.com/jackc/pgx/v5"
 )
 
@@ -238,3 +239,75 @@ func TestProposePlan_RefuseInFlight(t *testing.T) {
 		t.Fatalf("revised step generation = %d, want 1 (fresh-start after DELETE resets generation)", revisedSteps[0].Generation)
 	}
 }
+
+// TestHadDiscoveryAndWriteback is the store-level proof for D.1 (refuse
+// complete_task when discovery ran without writeback). hadDiscovery must
+// report true only after a successful `run` call; hadEntityWriteback must
+// report true only after a successful update_entity_attributes or
+// create_relationship call. The D.1 gate in tasks.go combines these: refuse
+// success when hadDiscovery && !hadEntityWriteback.
+func TestHadDiscoveryAndWriteback(t *testing.T) {
+	s := newTestStore(t)
+	ctx := context.Background()
+
+	sess, err := s.createSession(ctx, "discovery test")
+	if err != nil {
+		t.Fatalf("createSession: %v", err)
+	}
+
+	// Before any tool calls: no discovery, no writeback.
+	if s.hadDiscovery(ctx, sess.ID) {
+		t.Fatal("hadDiscovery = true before any tool calls, want false")
+	}
+	if s.hadEntityWriteback(ctx, sess.ID) {
+		t.Fatal("hadEntityWriteback = true before any tool calls, want false")
+	}
+
+	// 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")
+	if !s.hadDiscovery(ctx, sess.ID) {
+		t.Fatal("hadDiscovery = false after a successful run call, want true")
+	}
+	if s.hadEntityWriteback(ctx, sess.ID) {
+		t.Fatal("hadEntityWriteback = true after only a run call, want false")
+	}
+
+	// A failed run call should NOT count as discovery (no facts learned).
+	sess2, err := s.createSession(ctx, "failed discovery test")
+	if err != nil {
+		t.Fatalf("createSession: %v", err)
+	}
+	s.logActivity(ctx, agentID, sess2.ID, "run", nil, "", "ssh timeout", 100, false, "corr-2")
+	if s.hadDiscovery(ctx, sess2.ID) {
+		t.Fatal("hadDiscovery = true after a failed run call, want false (no facts learned)")
+	}
+
+	// A get_entity call should NOT count as discovery (DB lookup, not live state).
+	sess3, err := s.createSession(ctx, "lookup test")
+	if err != nil {
+		t.Fatalf("createSession: %v", err)
+	}
+	s.logActivity(ctx, agentID, sess3.ID, "get_entity", nil, "", "entity row", 10, true, "corr-3")
+	if s.hadDiscovery(ctx, sess3.ID) {
+		t.Fatal("hadDiscovery = true after get_entity, want false (DB lookups are not discovery)")
+	}
+
+	// update_entity_attributes sets hadEntityWriteback.
+	sess4, err := s.createSession(ctx, "writeback test")
+	if err != nil {
+		t.Fatalf("createSession: %v", err)
+	}
+	s.logActivity(ctx, agentID, sess4.ID, "update_entity_attributes", nil, "", "ok", 10, true, "corr-4")
+	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")
+	if !s.hadDiscovery(ctx, sess4.ID) {
+		t.Fatal("hadDiscovery = false after run+writeback, want true")
+	}
+	if !s.hadEntityWriteback(ctx, sess4.ID) {
+		t.Fatal("hadEntityWriteback = false after run+writeback, want true")
+	}
+}

cmd/nomos/tasks.go

@@ -37,14 +37,16 @@ func taskToolDefs() []toolDef {
 			Name: "propose_plan",
 			Description: "Propose the full ordered plan for this task. Call ONCE, before any " +
 				"execution, with EVERY step end-to-end (not one step at a time). FIRST step: " +
-				"research (prior knowledge, relations, blast radius). LAST step: write back " +
-				"(update_entity_attributes + create_relationship + upsert_knowledge) BEFORE " +
-				"complete_task — this keeps the knowledge graph from drifting. After this " +
-				"call: STOP and wait for operator approval (approval vocabulary: approved, " +
-				"yes, go, proceed, continue, ok, go ahead). Once a step has started " +
-				"(running/done/...), this tool REFUSES further calls — advance with " +
-				"update_plan_step + run instead. Re-propose only if the operator explicitly " +
-				"asks you to revise the whole plan.",
+				"research (prior knowledge, relations, blast radius). If your plan runs `run` " +
+				"against any target, include a LAST step: write back " +
+				"(update_entity_attributes + create_relationship + upsert_knowledge) — if you " +
+				"omit it, one is auto-appended. After this call: STOP and wait for operator " +
+				"approval (approval vocabulary: approved, yes, go, proceed, continue, ok, " +
+				"go ahead). Once a step has started (running/done/...), this tool REFUSES " +
+				"further calls — advance with update_plan_step + run instead. Re-propose only " +
+				"if the operator explicitly asks you to revise the whole plan. complete_task " +
+				"with outcome=success is REFUSED if you ran `run` but didn't call " +
+				"update_entity_attributes/create_relationship — write back before completing.",
 			InputSchema: map[string]any{
 				"type": "object",
 				"properties": map[string]any{
@@ -200,6 +202,32 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
 		if len(steps) == 0 {
 			return "error: propose_plan needs at least one step with a title", true
 		}
+		// D.2: auto-append a writeback step if the agent didn't include one.
+		// The agent consistently writes vague last steps ("record findings")
+		// and then skips update_entity_attributes entirely (the #1 cause of
+		// knowledge-graph drift). Appending an explicit writeback step makes
+		// the seq-order enforcement (5.6) require it to be completed last,
+		// and D.1's complete_task gate enforces the actual calls. Together
+		// they close the loop structurally — neither relies on the agent
+		// reading SOUL.md.
+		hasWritebackStep := false
+		for _, st := range steps {
+			if strings.Contains(st.Title, "update_entity_attributes") ||
+				strings.Contains(st.Title, "create_relationship") ||
+				strings.Contains(st.Detail, "update_entity_attributes") ||
+				strings.Contains(st.Detail, "create_relationship") {
+				hasWritebackStep = true
+				break
+			}
+		}
+		appendedNote := ""
+		if !hasWritebackStep {
+			steps = append(steps, planStepInput{
+				Title:  "Write back: update_entity_attributes + create_relationship + upsert_knowledge",
+				Detail: "Call update_entity_attributes for every entity you ran against (versions, states, counts, timestamps). Call create_relationship for any edge you discovered. Then upsert_knowledge about the affected entities (pass `about` as an array).",
+			})
+			appendedNote = fmt.Sprintf(" (appended a writeback step — your plan didn't include one; step %d)", len(steps))
+		}
 		persisted, err := a.store.proposePlan(ctx, sessionID, steps)
 		if err != nil {
 			if errors.Is(err, errPlanInFlight) {
@@ -213,15 +241,11 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
 			}
 			return fmt.Sprintf("error proposing plan: %v", err), 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 steps). STOP. Wait for operator approval — do not call run yet. Approval vocabulary: \"approved\", \"yes\", \"go\", \"proceed\", \"continue\", \"ok\", \"go ahead\". On approval, advance with update_plan_step + run. Do not call propose_plan again.", 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."
-		}
+		// The writeback step is now always present (D.2 auto-appends it if
+		// the agent forgot), so the old advisory nudge is replaced by the
+		// structural gate: D.1 refuses complete_task without the actual
+		// update_entity_attributes/create_relationship calls.
+		result := fmt.Sprintf("Plan set (%d steps)%s. STOP. Wait for operator approval — do not call run yet. Approval vocabulary: \"approved\", \"yes\", \"go\", \"proceed\", \"continue\", \"ok\", \"go ahead\". On approval, advance with update_plan_step + run. Do not call propose_plan again.", len(persisted), appendedNote)
 		return result, true
 
 	case "update_plan_step":
@@ -278,6 +302,17 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args
 				"session", sessionID, "outcome", outcome)
 			outcome = "partial"
 		}
+		// D.1: refuse success when discovery ran but no writeback followed.
+		// The prior advisory warning (below) was ignorable — the agent
+		// saw it and ended the task anyway. This gate fires BEFORE
+		// completeTask runs, so the session stays in 'executing' state
+		// and the agent must call update_entity_attributes/create_relationship
+		// then retry complete_task. Only blocks `success`; an explicit
+		// `failure` or `partial` is allowed through (the agent is
+		// acknowledging it didn't finish — no reason to force writeback).
+		if outcome == "success" && a.store.hadDiscovery(ctx, sessionID) && !a.store.hadEntityWriteback(ctx, sessionID) {
+			return "Refused: this session ran `run` against live targets (discovery) but did not call update_entity_attributes or create_relationship to persist what you learned. The knowledge graph will drift if you complete without writeback. Call update_entity_attributes for each entity you ran against (versions, states, counts, timestamps), and create_relationship for any edge you discovered, then call complete_task again. Outcome is held at 'executing' until you do.", true
+		}
 		if err := a.store.completeTask(ctx, sessionID, outcome, summary); err != nil {
 			return fmt.Sprintf("error completing task: %v", err), true
 		}