Files
oikos/cmd/nomos/store_test.go
dtoro 544afae77f
Some checks failed
ci / build-test (push) Has been cancelled
ci / docker-build (push) Has been cancelled
ci / web (push) Has been cancelled
Desktop App / Build Linux (amd64) (push) Has been cancelled
Desktop App / Attach to Release (push) Has been cancelled
feat(nomos): retry cap, vm: targets, inspect_path, goal supersession, runbooks
Session-review implementation for the three sessions audited in
plans/2026-07-18-session-review-three-sessions.md. v0.7.11 → v0.7.12.

P0.1 — retry cap + investigate-before-retry (cmd/nomos/retrycap.go,
agent.go): after 3 identical failing run calls in a single turn, refuse
to dispatch the call again and return a directive to investigate *why*
(ps/strace/lsof) or surface the blocker. Per-turn scope so a fresh turn
after the operator responds can retry once more. Session 1e9c7691's 20+
identical chown retries (knfsd held a kernel lock on the exported NFS
dir) is the direct motivation.

P0.2 + P1.8 + P2.10 — SOUL.md guidance: hung command is not a failed
command (investigate before retry); ask before proposing a multi-step
migration; multi-goal sessions summarize the arc not just the last goal.

P1.3 — two new runbook entities in seeds/knowledge.yaml:
  - nfs-exported-dir-mutation-hang (the knfsd fchownat lock procedure:
    killall → exportfs -u → mutate → exportfs -a → verify)
  - netbird-mgmt-oidc-race-after-upgrade (docker restart netbird-mgmt
    after ~30s for the traefik/authentik OIDC race)

P1.4 — setGoal emits task.superseded event when prior goal is overwritten
by a different goal (store.go, TestSetGoal_SupersededEvent). Session
55927f0a had two set_goal calls with the first silently abandoned.

P1.5 — inspect_path MCP tool: runs mount/df/ls/stat for one path across
up to 8 targets in one parallel call, replacing the 15+ run-call
fact-gathering fan-out sessions 1 and 2 each spent on cross-target path
tracing (tools.go, server.go: inspectPathAcrossTargets, inspectOneTarget).

P1.6 — vm: target support in run via qm guest exec (no more SSH-hop
with nested quoting). Extracted shared resolveProxmoxHostSlug for
LXC + VM, with hosts-relationship fallback when attributes.host is
absent (server.go, tools.go). Session 55927f0a's SSH-hop workarounds
for vm:zimaos are the direct motivation.

Deferred (documented in plan): P1.7 (approval window auto-extend on
timeout) and P2.9 (long-running command PENDING detection) — both
addressed at lower cost by the retry cap. Session 3's poll-after-timeout
pattern already works; the cap protects against the failure mode.
2026-07-19 00:09:39 +02:00

380 lines
14 KiB
Go

package main
// Integration tests against a real Postgres, mirroring
// internal/db/integration_test.go's pattern: guarded by
// OIKOS_TEST_DATABASE_URL (skipped when unset), throwaway database per run,
// full migrations applied, dropped on cleanup. Run with:
//
// docker compose up -d postgres
// OIKOS_TEST_DATABASE_URL="postgres://oikos:oikos_dev@localhost:5432/oikos?sslmode=disable" go test ./cmd/nomos/
import (
"context"
"errors"
"fmt"
"math/rand"
"os"
"strings"
"testing"
"github.com/dtoro/oikos/internal/db"
"github.com/google/uuid"
"github.com/jackc/pgx/v5"
)
// newTestStore creates a throwaway, fully-migrated database and returns a
// *store connected to it, cleaned up (including a matching task:<session>
// entity type in the ontology, needed by createTaskEntity/proposePlan tests)
// via t.Cleanup.
func newTestStore(t *testing.T) *store {
t.Helper()
baseURL := os.Getenv("OIKOS_TEST_DATABASE_URL")
if baseURL == "" {
t.Skip("OIKOS_TEST_DATABASE_URL not set — skipping integration test")
}
ctx := context.Background()
admin, err := pgx.Connect(ctx, baseURL)
if err != nil {
t.Fatalf("connect admin: %v", err)
}
dbName := fmt.Sprintf("oikos_test_nomos_%08x", rand.Int63())
if _, err := admin.Exec(ctx, "CREATE DATABASE "+dbName); err != nil {
admin.Close(ctx)
t.Fatalf("create test db: %v", err)
}
admin.Close(ctx)
testURL := swapTestDatabase(baseURL, dbName)
pool, err := db.New(ctx, testURL)
if err != nil {
t.Fatalf("connect test db: %v", err)
}
t.Cleanup(func() {
pool.Close()
admin, err := pgx.Connect(ctx, baseURL)
if err == nil {
admin.Exec(ctx, "DROP DATABASE IF EXISTS "+dbName+" WITH (FORCE)")
admin.Close(ctx)
}
})
if err := pool.Migrate(ctx); err != nil {
t.Fatalf("migrate: %v", err)
}
// session_plan_steps/session_questions tests don't need the ontology
// seed, but createTaskEntity's INSERT INTO entities (type='task') has an
// FK to entity_types — seed the minimal rows it needs directly rather
// than pulling in the full seeds/ontology.yaml ingest path.
if _, err := pool.Exec(ctx, `
INSERT INTO entity_types (name, domain, layer) VALUES ('entity', 'meta', 'meta')
ON CONFLICT DO NOTHING;
INSERT INTO entity_types (name, parent_type, domain, layer) VALUES ('task', 'entity', 'cognition', 'cognition')
ON CONFLICT DO NOTHING;`); err != nil {
t.Fatalf("seed minimal ontology: %v", err)
}
return &store{pool: pool.Pool}
}
func swapTestDatabase(url, dbName string) string {
qi := strings.Index(url, "?")
params, base := "", url
if qi >= 0 {
params = url[qi:]
base = url[:qi]
}
si := strings.LastIndex(base, "/")
return base[:si+1] + dbName + params
}
// TestGetRecentMessages_Truncation is the concrete proof for fix A2 of
// plans/2026-07-11-nomos-agent-code-review.md: chatWith used to replay a
// session's ENTIRE history on every turn with no bound. getRecentMessages
// caps that; this test checks both sides — under the limit, nothing is
// dropped and truncated=false; over it, only the most recent `limit` come
// back, in chronological order, with truncated=true.
func TestGetRecentMessages_Truncation(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
sess, err := s.createSession(ctx, "history window test")
if err != nil {
t.Fatalf("createSession: %v", err)
}
const total = 35
const limit = 30
for i := 0; i < total; i++ {
role := "user"
if i%2 == 1 {
role = "assistant"
}
body := fmt.Appendf(nil, `{"role":%q,"text":"msg-%d"}`, role, i)
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)
if err != nil {
t.Fatalf("getRecentMessages: %v", err)
}
if !truncated {
t.Errorf("truncated = false, want true (%d messages > limit %d)", total, limit)
}
if len(msgs) != limit {
t.Fatalf("got %d messages, want %d", len(msgs), limit)
}
// Chronological order: the oldest of the RETAINED messages should be the
// (total-limit)-th one saved (msg-5, since msg-0..4 were dropped), and
// 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 {
t.Errorf("first retained message = %q, want %q", got, wantFirst)
}
if got := extractText(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")
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 {
t.Fatalf("saveMessage: %v", err)
}
}
msgs2, truncated2, err := s.getRecentMessages(ctx, sess2.ID, limit)
if err != nil {
t.Fatalf("getRecentMessages (small): %v", err)
}
if truncated2 {
t.Errorf("truncated = true for a 5-message session under a %d limit, want false", limit)
}
if len(msgs2) != 5 {
t.Errorf("got %d messages, want 5", len(msgs2))
}
}
// TestProposePlan_RefuseInFlight is the concrete proof for the plan-drift
// fix (2026-07-14, "plan added twice in the sidebar"): proposePlan must
// REPLACE the step list only while every existing step is still 'pending'
// (a genuine pre-execution revision), and REFUSE the call once any step has
// started. The prior append-mode safety net (commit 5384499) preserved
// history but duplicated the plan in the sidebar when the agent re-proposed
// on "proceed". Refusing is the correct default — the agent must advance
// with update_plan_step + run.
func TestProposePlan_RefuseInFlight(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
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"}})
if err != nil {
t.Fatalf("proposePlan #1: %v", err)
}
if len(out1) != 1 || out1[0]["seq"] != 1 {
t.Fatalf("proposePlan #1 = %+v, want one step at seq 1", out1)
}
if out1[0]["generation"] != 1 {
t.Fatalf("proposePlan #1 generation = %v, want 1", out1[0]["generation"])
}
// Mark step 1 as started.
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)
}
// The original step 1 must be untouched — not erased, not appended to.
steps, err := s.getPlanSteps(ctx, sess.ID)
if err != nil {
t.Fatalf("getPlanSteps: %v", err)
}
if len(steps) != 1 {
t.Fatalf("got %d persisted steps, want 1 (refused call must not mutate the plan)", len(steps))
}
if steps[0].Title != "Step A" || steps[0].Status != "running" {
t.Errorf("step 1 = %+v, want Step A still running (refused call must not touch it)", steps[0])
}
// Third call BEFORE anything runs on a fresh session: every step is
// still pending, so this must REPLACE, not refuse.
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 {
t.Fatalf("proposePlan (initial): %v", err)
}
if _, err := s.proposePlan(ctx, sess2.ID, []planStepInput{{Title: "Revised"}}); err != nil {
t.Fatalf("proposePlan (revise before execution): %v", err)
}
revisedSteps, err := s.getPlanSteps(ctx, sess2.ID)
if err != nil {
t.Fatalf("getPlanSteps: %v", err)
}
if len(revisedSteps) != 1 || revisedSteps[0].Title != "Revised" {
t.Fatalf("got %+v, want a single 'Revised' step (pre-execution revise must replace, not refuse)", revisedSteps)
}
if revisedSteps[0].Generation != 1 {
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")
}
}
// TestSetGoal_SupersessionEvent is the store-level proof for P1.4 from
// plans/2026-07-18-session-review-three-sessions.md: when setGoal is called
// and a non-empty prior goal already exists with a DIFFERENT value, a
// task.superseded event must be emitted (so the audit trail records the
// pivot — the row's goal column will be overwritten, losing the prior intent
// without this event). When the goal is identical OR no prior goal exists,
// no supersession event is emitted.
//
// Background: session 55927f0a had two set_goal calls; the first was
// implicitly abandoned when the operator said "lets just keep ludo-library
// then." Without the event, the prior goal silently disappeared.
func TestSetGoal_SupersededEvent(t *testing.T) {
s := newTestStore(t)
ctx := context.Background()
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 {
t.Fatalf("setGoal #1: %v", err)
}
if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 0 {
t.Errorf("after first set_goal: %d task.superseded events, want 0", n)
}
// 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 {
t.Fatalf("setGoal #2: %v", err)
}
if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 1 {
t.Errorf("after second set_goal with a different goal: %d task.superseded events, want 1", n)
}
// 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 {
t.Fatalf("setGoal #3: %v", err)
}
if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 1 {
t.Errorf("after third set_goal with same goal as second: %d task.superseded events, want 1 (no new pivot)", n)
}
// The session's current goal must be the latest one set.
got, err := s.getSession(ctx, sess.ID)
if err != nil {
t.Fatalf("getSession: %v", err)
}
if got.Goal != "Add NFS export of ludo-lvm to ZimaOS" {
t.Errorf("session goal = %q, want the second (latest) goal", got.Goal)
}
}
// countEvents counts observability events of the given type correlated to
// the given session. Used by TestSetGoal_SupersededEvent to assert the
// task.superseded audit-trail signal was emitted.
func countEvents(ctx context.Context, s *store, sessionID, eventType string) int {
var n int
s.pool.QueryRow(ctx,
`SELECT COUNT(*) FROM events WHERE correlation_id = $1 AND type = $2`,
sessionID, eventType).Scan(&n)
return n
}