feat: Phase 9 gaps closed — ApprovalService.Decide convergence, execlog fold, execworker poller

- ApprovalService (core/app/approval.go) + ApprovalRepo (postgres adapter) with full decide transaction: HMAC token verify, approval flip, execution un-gate, session-scoped window keys (+session suffix matching GovernanceStore gate), nomos session flip, audit+event on failure abort. httpapi DecideApproval now a thin presenter delegating to the service. ListPending payload format fixed (json.Unmarshal not raw-wrap). - execlog folded into postgres adapter: internal/execlog deleted, NewExecutionLog / ReadExecutionLog live in the db package, callers updated (mcp, httpapi). - execworker poller over ExecutionService.DispatchQueued: advisory lock leak fixed (defer/recover per execution), correlation_id preserved via Finalize event emission (ExecRunRepo.Finalize now emits execution.{status} with correlation_id from the row). - Phase 8 session export-rename completed: Store, New, and all 53 methods exported; cmd/nomos/ agent.go fixed to use session.PendingContinuation etc. - Coverage gates: ExecutionService.Submit 93.1%, PolicyService.Decide 100%. - Plans index updated, VERSION bumped to 0.36.0.
2026-08-16 12:29:59 +02:00
parent 986937799a
commit b98d7c24bf
27 changed files with 1161 additions and 600 deletions
--- a/internal/nomos/session/store_test.go
+++ b/internal/nomos/session/store_test.go
@@ -1,4 +1,4 @@
-package main
+package session

 // Integration tests against a real Postgres, mirroring
 // internal/db/integration_test.go's pattern: guarded by
@@ -10,6 +10,7 @@ package main

 import (
 	"context"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"math/rand"
@@ -98,7 +99,7 @@ func TestGetRecentMessages_Truncation(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()

-	sess, err := s.createSession(ctx, "history window test")
+	sess, err := s.CreateSession(ctx, "history window test")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}
@@ -111,12 +112,12 @@ func TestGetRecentMessages_Truncation(t *testing.T) {
 			role = "assistant"
 		}
 		body := fmt.Appendf(nil, `{"role":%q,"text":"msg-%d"}`, role, i)
-		if err := s.saveMessage(ctx, sess.ID, role, body); err != nil {
+		if err := s.SaveMessage(ctx, sess.ID, role, body); err != nil {
 			t.Fatalf("saveMessage %d: %v", i, err)
 		}
 	}

-	msgs, truncated, err := s.getRecentMessages(ctx, sess.ID, limit)
+	msgs, truncated, err := s.GetRecentMessages(ctx, sess.ID, limit)
 	if err != nil {
 		t.Fatalf("getRecentMessages: %v", err)
 	}
@@ -131,25 +132,25 @@ func TestGetRecentMessages_Truncation(t *testing.T) {
 	// the last should be the most recently saved (msg-34).
 	wantFirst := fmt.Sprintf("msg-%d", total-limit)
 	wantLast := fmt.Sprintf("msg-%d", total-1)
-	if got := extractText(msgs[0].Content); got != wantFirst {
+	if got := sessionText(msgs[0].Content); got != wantFirst {
 		t.Errorf("first retained message = %q, want %q", got, wantFirst)
 	}
-	if got := extractText(msgs[len(msgs)-1].Content); got != wantLast {
+	if got := sessionText(msgs[len(msgs)-1].Content); got != wantLast {
 		t.Errorf("last retained message = %q, want %q", got, wantLast)
 	}

 	// Under the limit: nothing dropped.
-	sess2, err := s.createSession(ctx, "small session")
+	sess2, err := s.CreateSession(ctx, "small session")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}
 	for i := 0; i < 5; i++ {
 		body := fmt.Appendf(nil, `{"role":"user","text":"msg-%d"}`, i)
-		if err := s.saveMessage(ctx, sess2.ID, "user", body); err != nil {
+		if err := s.SaveMessage(ctx, sess2.ID, "user", body); err != nil {
 			t.Fatalf("saveMessage: %v", err)
 		}
 	}
-	msgs2, truncated2, err := s.getRecentMessages(ctx, sess2.ID, limit)
+	msgs2, truncated2, err := s.GetRecentMessages(ctx, sess2.ID, limit)
 	if err != nil {
 		t.Fatalf("getRecentMessages (small): %v", err)
 	}
@@ -173,14 +174,14 @@ func TestProposePlan_RefuseInFlight(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()

-	sess, err := s.createSession(ctx, "plan refuse test")
+	sess, err := s.CreateSession(ctx, "plan refuse test")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}

 	// First call: no steps exist yet — must persist as-is (replace mode,
 	// trivially: nothing to replace).
-	out1, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "Step A"}})
+	out1, err := s.ProposePlan(ctx, sess.ID, []PlanStepInput{{Title: "Step A"}})
 	if err != nil {
 		t.Fatalf("proposePlan #1: %v", err)
 	}
@@ -192,20 +193,20 @@ 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)
 	}

 	// Second call, simulating a model that re-proposes mid-flight (the
 	// operator-reported "proceed" bug): since step 1 has left 'pending',
-	// this MUST refuse with errPlanInFlight, not append or replace.
-	_, err = s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "Step B"}})
-	if !errors.Is(err, errPlanInFlight) {
-		t.Fatalf("proposePlan #2: err = %v, want errPlanInFlight (refuse mid-flight re-proposal)", err)
+	// this MUST refuse with ErrPlanInFlight, not append or replace.
+	_, err = s.ProposePlan(ctx, sess.ID, []PlanStepInput{{Title: "Step B"}})
+	if !errors.Is(err, ErrPlanInFlight) {
+		t.Fatalf("proposePlan #2: err = %v, want ErrPlanInFlight (refuse mid-flight re-proposal)", err)
 	}

 	// The original step 1 must be untouched — not erased, not appended to.
-	steps, err := s.getPlanSteps(ctx, sess.ID, false)
+	steps, err := s.GetPlanSteps(ctx, sess.ID, false)
 	if err != nil {
 		t.Fatalf("getPlanSteps: %v", err)
 	}
@@ -219,18 +220,18 @@ func TestProposePlan_RefuseInFlight(t *testing.T) {
 	// Third call BEFORE anything runs on a fresh session: every step is
 	// still pending, so this must REPLACE (mark the prior plan `replaced`),
 	// not refuse. The new plan becomes generation 2.
-	sess2, err := s.createSession(ctx, "plan replace test")
+	sess2, err := s.CreateSession(ctx, "plan replace test")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}
-	if _, err := s.proposePlan(ctx, sess2.ID, []PlanStepInput{{Title: "Original"}}); err != nil {
+	if _, err := s.ProposePlan(ctx, sess2.ID, []PlanStepInput{{Title: "Original"}}); err != nil {
 		t.Fatalf("proposePlan (initial): %v", err)
 	}
-	if _, err := s.proposePlan(ctx, sess2.ID, []PlanStepInput{{Title: "Revised"}}); err != nil {
+	if _, err := s.ProposePlan(ctx, sess2.ID, []PlanStepInput{{Title: "Revised"}}); err != nil {
 		t.Fatalf("proposePlan (revise before execution): %v", err)
 	}
 	// Default (current generation) view: only the revised step.
-	revisedSteps, err := s.getPlanSteps(ctx, sess2.ID, false)
+	revisedSteps, err := s.GetPlanSteps(ctx, sess2.ID, false)
 	if err != nil {
 		t.Fatalf("getPlanSteps: %v", err)
 	}
@@ -244,7 +245,7 @@ func TestProposePlan_RefuseInFlight(t *testing.T) {
 		t.Fatalf("revised step generation = %d, want 2 (prior pending plan is replaced, not deleted, so the counter increments)", revisedSteps[0].Generation)
 	}
 	// all=true audit view: both generations, the original marked `replaced`.
-	allSteps, err := s.getPlanSteps(ctx, sess2.ID, true)
+	allSteps, err := s.GetPlanSteps(ctx, sess2.ID, true)
 	if err != nil {
 		t.Fatalf("getPlanSteps(all): %v", err)
 	}
@@ -270,32 +271,32 @@ func TestUpdatePlanStep_GenerationRelative(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()

-	sess, err := s.createSession(ctx, "gen-relative seq test")
+	sess, err := s.CreateSession(ctx, "gen-relative seq test")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}
 	// Generation 1: two steps.
-	if _, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "A"}, {Title: "B"}}); err != nil {
+	if _, err := s.ProposePlan(ctx, sess.ID, []PlanStepInput{{Title: "A"}, {Title: "B"}}); err != nil {
 		t.Fatalf("proposePlan #1: %v", err)
 	}
 	// Re-plan: setGoal marks the gen-1 plan `replaced`, proposePlan starts gen 2.
-	if err := s.setGoal(ctx, sess.ID, "follow-up sub-task"); err != nil {
+	if err := s.SetGoal(ctx, sess.ID, "follow-up sub-task"); err != nil {
 		t.Fatalf("setGoal: %v", err)
 	}
-	if _, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "C"}, {Title: "D"}}); err != nil {
+	if _, err := s.ProposePlan(ctx, sess.ID, []PlanStepInput{{Title: "C"}, {Title: "D"}}); err != nil {
 		t.Fatalf("proposePlan #2: %v", err)
 	}

 	// 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)
 	}

-	all, err := s.getPlanSteps(ctx, sess.ID, true)
+	all, err := s.GetPlanSteps(ctx, sess.ID, true)
 	if err != nil {
 		t.Fatalf("getPlanSteps(all): %v", err)
 	}
@@ -319,8 +320,8 @@ 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) {
-		t.Fatalf("updatePlanStep(seq=99) err = %v, want errPlanStepNotFound", err)
+	if err := s.UpdatePlanStep(ctx, sess.ID, 99, "running", "", ""); !errors.Is(err, ErrPlanStepNotFound) {
+		t.Fatalf("updatePlanStep(seq=99) err = %v, want ErrPlanStepNotFound", err)
 	}
 }

@@ -333,24 +334,24 @@ func TestCompleteTask_AutoCloseEmitsEvents(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()

-	sess, err := s.createSession(ctx, "auto-close events test")
+	sess, err := s.CreateSession(ctx, "auto-close events test")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}
-	if _, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "A"}, {Title: "B"}}); err != nil {
+	if _, err := s.ProposePlan(ctx, sess.ID, []PlanStepInput{{Title: "A"}, {Title: "B"}}); err != nil {
 		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 {
+	if err := s.CompleteTask(ctx, sess.ID, "success", "done"); err != nil {
 		t.Fatalf("completeTask: %v", err)
 	}

 	// Every auto-closed step should now carry both a started_at and a
 	// finished_at (no NULL-started `done` step).
-	steps, err := s.getPlanSteps(ctx, sess.ID, true)
+	steps, err := s.GetPlanSteps(ctx, sess.ID, true)
 	if err != nil {
 		t.Fatalf("getPlanSteps: %v", err)
 	}
@@ -382,64 +383,64 @@ func TestHadDiscoveryAndWriteback(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()

-	sess, err := s.createSession(ctx, "discovery test")
+	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) {
+	if s.HadDiscovery(ctx, sess.ID) {
 		t.Fatal("hadDiscovery = true before any tool calls, want false")
 	}
-	if s.hadEntityWriteback(ctx, sess.ID) {
+	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", 0)
-	if !s.hadDiscovery(ctx, sess.ID) {
+	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")
 	}
-	if s.hadEntityWriteback(ctx, sess.ID) {
+	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")
+	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", 0)
-	if s.hadDiscovery(ctx, sess2.ID) {
+	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)")
 	}

 	// A get_entity call should NOT count as discovery (DB lookup, not live state).
-	sess3, err := s.createSession(ctx, "lookup test")
+	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", 0)
-	if s.hadDiscovery(ctx, sess3.ID) {
+	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)")
 	}

 	// update_entity_attributes sets hadEntityWriteback.
-	sess4, err := s.createSession(ctx, "writeback test")
+	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", 0)
-	if !s.hadEntityWriteback(ctx, sess4.ID) {
+	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", 0)
-	if !s.hadDiscovery(ctx, sess4.ID) {
+	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")
 	}
-	if !s.hadEntityWriteback(ctx, sess4.ID) {
+	if !s.HadEntityWriteback(ctx, sess4.ID) {
 		t.Fatal("hadEntityWriteback = false after run+writeback, want true")
 	}
 }
@@ -459,13 +460,13 @@ func TestSetGoal_SupersededEvent(t *testing.T) {
 	s := newTestStore(t)
 	ctx := context.Background()

-	sess, err := s.createSession(ctx, "goal pivot test")
+	sess, err := s.CreateSession(ctx, "goal pivot test")
 	if err != nil {
 		t.Fatalf("createSession: %v", err)
 	}

 	// First set_goal — no prior, no supersession event expected.
-	if err := s.setGoal(ctx, sess.ID, "Fix sabnzbd download folder to use ludo-lvm"); err != nil {
+	if err := s.SetGoal(ctx, sess.ID, "Fix sabnzbd download folder to use ludo-lvm"); err != nil {
 		t.Fatalf("setGoal #1: %v", err)
 	}
 	if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 0 {
@@ -473,7 +474,7 @@ func TestSetGoal_SupersededEvent(t *testing.T) {
 	}

 	// Second set_goal with a DIFFERENT goal — supersession event expected.
-	if err := s.setGoal(ctx, sess.ID, "Add NFS export of ludo-lvm to ZimaOS"); err != nil {
+	if err := s.SetGoal(ctx, sess.ID, "Add NFS export of ludo-lvm to ZimaOS"); err != nil {
 		t.Fatalf("setGoal #2: %v", err)
 	}
 	if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 1 {
@@ -482,7 +483,7 @@ func TestSetGoal_SupersededEvent(t *testing.T) {

 	// Third set_goal with the SAME goal as the second — no new supersession
 	// event (idempotent: same goal is a no-op, not a pivot).
-	if err := s.setGoal(ctx, sess.ID, "Add NFS export of ludo-lvm to ZimaOS"); err != nil {
+	if err := s.SetGoal(ctx, sess.ID, "Add NFS export of ludo-lvm to ZimaOS"); err != nil {
 		t.Fatalf("setGoal #3: %v", err)
 	}
 	if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 1 {
@@ -490,7 +491,7 @@ func TestSetGoal_SupersededEvent(t *testing.T) {
 	}

 	// The session's current goal must be the latest one set.
-	got, err := s.getSession(ctx, sess.ID)
+	got, err := s.GetSession(ctx, sess.ID)
 	if err != nil {
 		t.Fatalf("getSession: %v", err)
 	}
@@ -509,3 +510,14 @@ func countEvents(ctx context.Context, s *Store, sessionID, eventType string) int
 		sessionID, eventType).Scan(&n)
 	return n
 }
+
+// sessionText pulls the "text" field from a persisted message's JSONB content.
+func sessionText(content json.RawMessage) string {
+	var m struct {
+		Text string `json:"text"`
+	}
+	if err := json.Unmarshal(content, &m); err != nil {
+		return ""
+	}
+	return m.Text
+}