From 7a7d390718de80bef1b017dc26cea5ee0cfed897 Mon Sep 17 00:00:00 2001 From: dtoro Date: Sun, 16 Aug 2026 10:31:45 +0200 Subject: [PATCH] =?UTF-8?q?feat:=20Phase=208=20=E2=80=94=20nomos=20package?= =?UTF-8?q?s=20extracted=20into=20internal/nomos/{turngate,retrycap,messag?= =?UTF-8?q?equeue,assent,session}?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mechanical extraction of nomos internal components into plain-Go subpackages per the hexagonal plan (ADR 0016 §3.1 rule 3): turngate/ — per-session turn serialization (plan 2026-08-03 F1) retrycap/ — per-turn run retry cap (maxRunRetries=3) messagequeue/ — operator-message queue for busy-turn re-entry (F2) assent/ — chat-assent detection (isAssent, isTypedConfirmation, ExtractPendingApprovals), decoupled from agent via []string input instead of persistedCall session/ — store (chat sessions, plan execution, DB persistence), migration runner + local emitEvent to break adapter dependency internal/migrate/ — shared migration runner extracted from postgres pool, used by both the oikos postgres adapter and session tests. session package export-rename finishing touches remain; the four smaller packages compile with passing tests. Depguard rules and ADR-0016 leaf-note update deferred to a followup. VERSION 0.35.1. --- VERSION | 2 +- cmd/nomos/agent.go | 105 +++++----- cmd/nomos/continue.go | 37 ++-- cmd/nomos/continue_test.go | 4 +- cmd/nomos/messagequeue_test.go | 142 ------------- cmd/nomos/server.go | 62 +++--- cmd/nomos/tasks.go | 48 ++--- cmd/nomos/workers.go | 18 +- internal/adapters/postgres/pool.go | 196 +----------------- internal/migrate/migrate.go | 119 +++++++++++ internal/migrate/split.go | 93 +++++++++ .../split_test.go} | 24 +-- .../nomos => internal/nomos/assent}/assent.go | 66 ++---- .../nomos/assent}/assent_test.go | 32 ++- .../nomos/messagequeue}/messagequeue.go | 40 ++-- .../nomos/messagequeue/messagequeue_test.go | 98 +++++++++ .../nomos/retrycap}/retrycap.go | 48 ++--- .../nomos/retrycap}/retrycap_test.go | 58 +++--- .../nomos => internal/nomos/session}/store.go | 0 .../nomos/session}/store_test.go | 26 +-- .../nomos/turngate}/turngate.go | 28 +-- .../nomos/turngate}/turngate_test.go | 66 +++--- 22 files changed, 629 insertions(+), 683 deletions(-) delete mode 100644 cmd/nomos/messagequeue_test.go create mode 100644 internal/migrate/migrate.go create mode 100644 internal/migrate/split.go rename internal/{adapters/postgres/splitsql_test.go => migrate/split_test.go} (84%) rename {cmd/nomos => internal/nomos/assent}/assent.go (73%) rename {cmd/nomos => internal/nomos/assent}/assent_test.go (74%) rename {cmd/nomos => internal/nomos/messagequeue}/messagequeue.go (64%) create mode 100644 internal/nomos/messagequeue/messagequeue_test.go rename {cmd/nomos => internal/nomos/retrycap}/retrycap.go (79%) rename {cmd/nomos => internal/nomos/retrycap}/retrycap_test.go (68%) rename {cmd/nomos => internal/nomos/session}/store.go (100%) rename {cmd/nomos => internal/nomos/session}/store_test.go (96%) rename {cmd/nomos => internal/nomos/turngate}/turngate.go (76%) rename {cmd/nomos => internal/nomos/turngate}/turngate_test.go (56%) diff --git a/VERSION b/VERSION index 7b52f5e5..731b95d7 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.35.0 +0.35.1 diff --git a/cmd/nomos/agent.go b/cmd/nomos/agent.go index 3703dbdb..0bde6b55 100644 --- a/cmd/nomos/agent.go +++ b/cmd/nomos/agent.go @@ -10,6 +10,11 @@ import ( "strings" "time" + "github.com/dtoro/oikos/internal/nomos/assent" + "github.com/dtoro/oikos/internal/nomos/messagequeue" + "github.com/dtoro/oikos/internal/nomos/retrycap" + "github.com/dtoro/oikos/internal/nomos/session" + "github.com/dtoro/oikos/internal/nomos/turngate" "github.com/google/uuid" "github.com/openai/openai-go" "github.com/openai/openai-go/option" @@ -51,7 +56,7 @@ type agent struct { system string provider *openai.Client model string - store *store + store *session.Store agentID uuid.UUID reqOpts []option.RequestOption apiBase string // oikos HTTP API base, derived from NOMOS_MCP_URL, for chat-assent approvals @@ -59,14 +64,14 @@ type agent struct { httpClient *http.Client // gate serializes turns per session (at most one in-flight turn per // sessionID). See turngate.go and plan 2026-08-03 F1. - gate *turnGate + gate *turngate.TurnGate // queue holds operator messages that arrived while a turn was already // running; they are auto-run when the gate frees (plan 2026-08-03 F2). // See messagequeue.go. - queue *messageQueue + queue *messagequeue.MessageQueue } -func newAgent(ctx context.Context, clients *mcpClientPool, st *store, agentSlug string, openrouterAPIKey string) (*agent, error) { +func newAgent(ctx context.Context, clients *mcpClientPool, st *session.Store, agentSlug string, openrouterAPIKey string) (*agent, error) { system := loadSoul() apiKey := openrouterAPIKey model := os.Getenv("NOMOS_MODEL") @@ -82,7 +87,7 @@ func newAgent(ctx context.Context, clients *mcpClientPool, st *store, agentSlug option.WithAPIKey(apiKey), ) - agentID := st.resolveAgentID(ctx, agentSlug) + agentID := st.ResolveAgentID(ctx, agentSlug) if agentID == uuid.Nil { slog.Warn("nomos: agent entity not found; tool-call activity will not be logged", "slug", agentSlug) } @@ -124,8 +129,8 @@ func newAgent(ctx context.Context, clients *mcpClientPool, st *store, agentSlug apiBase: apiBase, apiToken: os.Getenv("OIKOS_MCP_BEARER_TOKEN"), httpClient: &http.Client{Timeout: 15 * time.Second}, - gate: newTurnGate(), - queue: newMessageQueue(), + gate: turngate.New(), + queue: messagequeue.New(), }, nil } @@ -155,12 +160,12 @@ const assentWindowDuration = 30 * time.Minute // session dimension, approving one task's plan would silently auto-run // unapproved actions in any other concurrently-running task. func (a *agent) openAssentWindow(ctx context.Context, sessionID string) { - if a.store == nil || a.store.pool == nil || a.agentID == uuid.Nil || sessionID == "" { + if a.store == nil || a.agentID == uuid.Nil || sessionID == "" { return } - key := assentWindowKey(a.agentID, sessionID) + key := session.AssentWindowKey(a.agentID, sessionID) expires := time.Now().Add(assentWindowDuration).UTC().Format(time.RFC3339) - _, err := a.store.pool.Exec(ctx, + _, err := a.store.Exec(ctx, `INSERT INTO autonomy_settings (key, value) VALUES ($1, $2) ON CONFLICT (key) DO UPDATE SET value = $2`, key, expires) if err != nil { @@ -233,7 +238,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s system += "\n\n" + snapshot } messages := []openai.ChatCompletionMessageParamUnion{openai.SystemMessage(system)} - history, truncatedHistory, _ := a.store.getRecentMessages(ctx, sessionID, historyWindowSize) + history, truncatedHistory, _ := a.store.GetRecentMessages(ctx, sessionID, historyWindowSize) if truncatedHistory { // Tell the model explicitly rather than silently dropping older // turns — otherwise it might assume something wasn't done just @@ -289,36 +294,42 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s // actions are never granted by loose assent — they need the stricter // isTypedConfirmation ("I confirm ...", per SOUL.md's guidance for what // to ask the operator to type). - pending := extractPendingApprovals(lastAssistantCalls) - assent := isAssent(message) - typedConfirm := isTypedConfirmation(message) - if len(pending) > 0 && (assent || typedConfirm) { + pending := assent.ExtractPendingApprovals(func() []string { + texts := make([]string, len(lastAssistantCalls)) + for i, c := range lastAssistantCalls { + texts[i] = c.resultText() + } + return texts + }()) + operatorAssented := assent.IsAssent(message) + typedConfirm := assent.IsTypedConfirmation(message) + if len(pending) > 0 && (operatorAssented || typedConfirm) { var granted, blocked []string for _, p := range pending { - if p.destructive && !typedConfirm { - blocked = append(blocked, p.execID) + if p.Destructive && !typedConfirm { + blocked = append(blocked, p.ExecID) continue } - if !p.destructive && !assent { + if !p.Destructive && !operatorAssented { continue // typed-confirm alone doesn't grant a non-destructive item without also reading as assent } - ok, status, aerr := a.approveExecution(ctx, p.execID) + ok, status, aerr := a.approveExecution(ctx, p.ExecID) if aerr != nil { - slog.Error("nomos: chat-assent approve", "execution", p.execID, "error", aerr) + slog.Error("nomos: chat-assent approve", "execution", p.ExecID, "error", aerr) continue } if ok { - granted = append(granted, p.execID) - slog.Info("nomos: chat-assent granted", "execution", p.execID, "status", status, "session", sessionID) + granted = append(granted, p.ExecID) + slog.Info("nomos: chat-assent granted", "execution", p.ExecID, "status", status, "session", sessionID) // An explicit typed confirmation for a destructive action // opens a short, target-scoped window so the rest of a // destructive recovery sequence on the SAME target (e.g. // stop -> destroy) doesn't need a second typed confirmation. - if p.destructive && typedConfirm { - if execUUID, perr := uuid.Parse(p.execID); perr == nil { - if target := a.store.executionTarget(ctx, execUUID); target != "" { - a.store.openDestructiveWindow(ctx, a.agentID, target, sessionID) + if p.Destructive && typedConfirm { + if execUUID, perr := uuid.Parse(p.ExecID); perr == nil { + if target := a.store.ExecutionTarget(ctx, execUUID); target != "" { + a.store.OpenDestructiveWindow(ctx, a.agentID, target, sessionID) slog.Info("nomos: destructive window opened", "agent", a.agentID, "target", target, "session", sessionID) } } @@ -333,7 +344,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s // same session cause empty responses and race conditions. for _, execID := range granted { if execUUID, perr := uuid.Parse(execID); perr == nil { - a.store.markContinued(ctx, execUUID) + a.store.MarkContinued(ctx, execUUID) } } // No system note. The model already sees "go ahead" in the @@ -348,7 +359,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s note := fmt.Sprintf("[System: execution(s) %s are classified DESTRUCTIVE and were NOT approved by loose assent — you must ask the operator for an explicit typed confirmation before they can run. Once they do confirm, further destructive steps on that SAME target (e.g. finishing a stop-then-destroy sequence) will auto-run for 15 minutes without asking again — but a different target always needs its own confirmation.]", strings.Join(blocked, ", ")) messages = append(messages, openai.SystemMessage(note)) } - } else if assent && len(pending) == 0 { + } else if operatorAssented && len(pending) == 0 { // The operator said "proceed"/"go ahead"/"yes" but there are no // pending approvals — the agent proposed a plan (via propose_plan) // and asked "shall I?" Open the assent window silently. No system @@ -365,12 +376,12 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s // Retry cap (P0.1 from plans/2026-07-18-session-review-three-sessions.md): // track failing `run` calls within this turn so an identical command that - // keeps failing is refused after maxRunRetries attempts. Without this, + // keeps failing is refused after retrycap.MaxRunRetries attempts. Without this, // session 1e9c7691 retried the same `chown` ~20 times, each retry piling // up a zombie process on the target (knfsd was holding a kernel lock). // The tracker is per-turn — a fresh turn after the operator responds can // retry once more, so this doesn't permanently block recovery. - retries := newRunRetryTracker() + retries := retrycap.New() for i := 0; i < maxIterations; i++ { params := openai.ChatCompletionNewParams{ @@ -501,7 +512,7 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s } // Retry cap: if this `run` call has already failed - // maxRunRetries times this turn with the same (target, + // retrycap.MaxRunRetries times this turn with the same (target, // command), refuse to dispatch it again. Return a synthetic // tool result directing the agent to investigate *why* the // command hangs instead of retrying. See retrycap.go and @@ -509,12 +520,12 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s if tc.Function.Name == "run" { t, _ := args["target"].(string) c, _ := args["command"].(string) - key := runFailureKey(t, c) - if n := retries.failures(key); n >= maxRunRetries { - directive := runRetryDirective(t, c, n) + key := retrycap.RunFailureKey(t, c) + if n := retries.Failures(key); n >= retrycap.MaxRunRetries { + directive := retrycap.RunRetryDirective(t, c, n) 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, + a.store.LogActivity(ctx, a.agentID, sessionID, tc.Function.Name, args, tc.Function.Arguments, directive, 0, false, correlationID, totalTokens) emit(agentEvent{ Type: "tool_result", @@ -566,7 +577,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, totalTokens) + 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 @@ -576,9 +587,9 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s if tc.Function.Name == "run" { t, _ := args["target"].(string) c, _ := args["command"].(string) - key := runFailureKey(t, c) - n := retries.recordFailure(key) - if n >= maxRunRetries { + key := retrycap.RunFailureKey(t, c) + n := retries.RecordFailure(key) + if n >= retrycap.MaxRunRetries { slog.Warn("nomos: run failure cap reached — next identical call will be refused", "target", t, "failures", n, "session", sessionID) } @@ -596,7 +607,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, totalTokens) + 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 @@ -604,18 +615,18 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s // executions (pct_create, apt_upgrade) are the ones that matter — // their result lands after this turn ends. for _, execID := range extractExecutionIDs(string(resultJSON)) { - a.store.linkExecution(ctx, execID, sessionID) + a.store.LinkExecution(ctx, execID, sessionID) } // Record which entities this task touched (task —involves→ entity) // and pulse them on the live context panel. Args only — never // results — so a bulk query doesn't drag the whole fleet in. - a.store.recordTouched(ctx, sessionID, tc.Function.Name, args) + a.store.RecordTouched(ctx, sessionID, tc.Function.Name, args) // When the agent records knowledge, link that note to this task so // the task's outcome view shows what it learned (and pulse it live). if tc.Function.Name == "upsert_knowledge" { - a.store.linkKnowledgeToTask(ctx, sessionID, string(resultJSON)) + a.store.LinkKnowledgeToTask(ctx, sessionID, string(resultJSON)) } emit(agentEvent{ @@ -635,12 +646,12 @@ func (a *agent) chatWith(ctx context.Context, sessionID, message, systemInject s // queued for approval (pending approvals are not failures). // Pass the RAW result text (not JSON-encoded) so the helper's // HasPrefix check sees "run on …" not "\"run on …\"". - if isRunFailure(tc.Function.Name, runResultText(result), callErr) { + if retrycap.IsRunFailure(tc.Function.Name, retrycap.RunResultText(result), callErr) { t, _ := args["target"].(string) c, _ := args["command"].(string) - key := runFailureKey(t, c) - n := retries.recordFailure(key) - if n >= maxRunRetries { + key := retrycap.RunFailureKey(t, c) + n := retries.RecordFailure(key) + if n >= retrycap.MaxRunRetries { slog.Warn("nomos: run failure cap reached — next identical call will be refused", "target", t, "failures", n, "session", sessionID) } diff --git a/cmd/nomos/continue.go b/cmd/nomos/continue.go index bbaa16ce..b92c1046 100644 --- a/cmd/nomos/continue.go +++ b/cmd/nomos/continue.go @@ -9,6 +9,7 @@ import ( "strings" "time" + "github.com/dtoro/oikos/internal/nomos/session" "github.com/dtoro/oikos/internal/safego" "github.com/google/uuid" ) @@ -71,7 +72,7 @@ func (a *agent) runIdleSweepWorker(ctx context.Context) { // reasoning resumeSession already applies below for a different failure // mode (a resume that produces no response at all). func (a *agent) processIdleSweep(ctx context.Context) { - stale := a.store.staleGoalSessions(ctx, idleTaskThreshold, 5) + stale := a.store.StaleGoalSessions(ctx, idleTaskThreshold, 5) for _, s := range stale { s := s if s.CompletionNudges == 0 { @@ -80,13 +81,13 @@ func (a *agent) processIdleSweep(ctx context.Context) { "If the goal is done (or can't be completed), call complete_task now with the outcome and a "+ "one-line summary. If you're still genuinely working through the plan, ignore this and continue.]", s.Goal, idleTaskThreshold) - note = a.store.enrichResumeNote(ctx, s.ID, note) + note = a.store.EnrichResumeNote(ctx, s.ID, note) // P1: only count the nudge if it actually delivered. resumeSession // skips (returns false) when a turn is already active; bumping the // counter anyway would make the next sweep auto-close a merely-busy // session as "unanswered." if a.resumeSession(ctx, s.ID, note) { - if err := a.store.bumpCompletionNudge(ctx, s.ID); err != nil { + if err := a.store.BumpCompletionNudge(ctx, s.ID); err != nil { slog.Error("nomos: idle nudge bump failed", "session", s.ID, "error", err) } } @@ -95,7 +96,7 @@ func (a *agent) processIdleSweep(ctx context.Context) { } safego.Go("nomos:idle-autoclose:"+s.ID, func() { summary := fmt.Sprintf("Auto-closed after %s idle with no response to a completion nudge.", idleTaskThreshold) - if err := a.store.completeTask(ctx, s.ID, "partial", summary); err != nil { + if err := a.store.CompleteTask(ctx, s.ID, "partial", summary); err != nil { slog.Error("nomos: idle auto-close failed", "session", s.ID, "error", err) } }) @@ -141,19 +142,19 @@ func (a *agent) runContinuationWorker(ctx context.Context) { // silently die until nomos restarted. Now a single bad item can only ever // take down its own goroutine. func (a *agent) processContinuations(ctx context.Context) { - pending := a.store.pendingContinuations(ctx, 5) + pending := a.store.PendingContinuations(ctx, 5) for _, p := range pending { // Scope gate: only auto-continue while an approved plan is active FOR // THIS SESSION. Checked per-item, not once for the whole batch — with // multiple tasks in flight, one task's open window must never cover a // pending continuation belonging to a different task. - if !a.store.assentWindowActive(ctx, a.agentID, p.SessionID) { + if !a.store.AssentWindowActive(ctx, a.agentID, p.SessionID) { // Re-open the assent window if this session is genuinely // executing (plan was approved, work is in progress) — the // window may have expired while the execution ran. Don't // penalize timing: the plan was approved, the work happened, // the result should flow back. - sesh, seshErr := a.store.getSession(ctx, p.SessionID) + sesh, seshErr := a.store.GetSession(ctx, p.SessionID) if seshErr == nil && sesh.Goal != "" && (sesh.Status == "executing" || sesh.Status == "planning") { a.openAssentWindow(ctx, p.SessionID) slog.Info("nomos: re-opened assent window for continuing session", "session", p.SessionID, "execution", p.ExecID) @@ -162,8 +163,8 @@ func (a *agent) processContinuations(ctx context.Context) { // operator knows WHY the agent didn't auto-continue. note := fmt.Sprintf("[System: execution %s finished with status=%s, but the assent window for this session is not active. The agent will not auto-continue. Reply 'continue' or re-approve the plan to resume.]", p.ExecID, p.Status) body, _ := json.Marshal(map[string]any{"role": "assistant", "text": note, "auto": true}) - a.store.saveMessage(context.Background(), p.SessionID, "assistant", body) - a.store.markContinued(ctx, p.ExecID) + a.store.SaveMessage(context.Background(), p.SessionID, "assistant", body) + a.store.MarkContinued(ctx, p.ExecID) continue } } @@ -183,7 +184,7 @@ func (a *agent) processContinuations(ctx context.Context) { // that takes, which is exactly the "I just wait while nothing happens" // complaint this exists to fix — polling alone only helps if there's // something new to poll for. -func (a *agent) continueSession(ctx context.Context, p pendingContinuation) { +func (a *agent) continueSession(ctx context.Context, p session.PendingContinuation) { slog.Info("nomos: auto-continuing session", "session", p.SessionID, "execution", p.ExecID, "status", p.Status) // P0 (plans/2026-08-03-nomos-chat-changes-review.md): mark the execution // continued ONLY after the turn actually ran. resumeSession skips (returns @@ -196,7 +197,7 @@ func (a *agent) continueSession(ctx context.Context, p pendingContinuation) { slog.Info("nomos: continuation deferred — a turn is active; will retry next tick", "session", p.SessionID, "execution", p.ExecID) return } - a.store.markContinued(ctx, p.ExecID) + a.store.MarkContinued(ctx, p.ExecID) } // resumeSession re-invokes the agent for a session with a system-injected note — @@ -218,7 +219,7 @@ func (a *agent) continueSession(ctx context.Context, p pendingContinuation) { // the state changed but the work undone (lost continuation / false auto-close). // See plans/2026-08-03-nomos-chat-changes-review.md P0/P1. func (a *agent) resumeSession(ctx context.Context, sessionID, note string) bool { - if !a.gate.acquire(sessionID, 0) { + if !a.gate.Acquire(sessionID, 0) { slog.Info("nomos: turn already active, skipping background resume", "session", sessionID) return false } @@ -226,7 +227,7 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) bool // this background turn ran (plan 2026-08-03 F2). Queued messages are run as // real user turns server-side; resumeSession itself never enqueues. defer func() { - a.gate.release(sessionID) + a.gate.Release(sessionID) safego.Go("nomos:drain:"+sessionID, func() { a.drainQueued(context.Background(), sessionID) }) }() @@ -235,7 +236,7 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) bool "text": "", "auto": true, }) - msgID, err := a.store.insertMessageReturningID(ctx, sessionID, "assistant", placeholder) + msgID, err := a.store.InsertMessageReturningID(ctx, sessionID, "assistant", placeholder) if err != nil { slog.Error("nomos: resume placeholder insert failed", "session", sessionID, "error", err) } @@ -259,7 +260,7 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) bool "tool_calls": toolCalls, "auto": true, // marks this as an autonomous continuation, not an operator turn }) - a.store.updateMessage(ctx, msgID, body) + a.store.UpdateMessage(ctx, msgID, body) } // One retry if the LLM call itself produced nothing (transient flake / @@ -362,10 +363,10 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) bool "auto": true, }) if msgID != uuid.Nil { - a.store.updateMessage(context.Background(), msgID, body) + a.store.UpdateMessage(context.Background(), msgID, body) } else { // No placeholder was inserted (rare), save directly. - a.store.saveMessage(context.Background(), sessionID, "assistant", body) + a.store.SaveMessage(context.Background(), sessionID, "assistant", body) } return true // do not call persist() again — already persisted above } @@ -376,7 +377,7 @@ func (a *agent) resumeSession(ctx context.Context, sessionID, note string) bool // buildContinuationNote frames the finished execution for the model: what // happened, and what to do about it. The persist-through-errors instruction // lives here (and in SOUL) so the agent recovers instead of stopping. -func buildContinuationNote(p pendingContinuation) string { +func buildContinuationNote(p session.PendingContinuation) string { action := p.Action if i := strings.IndexByte(action, ':'); i > 0 && len(action) > 40 { action = action[:i] // keep just the action verb for brevity; params are in the DB diff --git a/cmd/nomos/continue_test.go b/cmd/nomos/continue_test.go index b1d99bd4..1ec06bc1 100644 --- a/cmd/nomos/continue_test.go +++ b/cmd/nomos/continue_test.go @@ -54,7 +54,7 @@ func TestExtractExecutionIDs(t *testing.T) { // would dereference the nil provider and panic. Returning false cleanly proves // the body was skipped. func TestResumeSession_SkipsWhenBusy(t *testing.T) { - a := &agent{gate: newTurnGate()} + a := &agent{gate: turngate.New()} if !a.gate.acquire("sess", 0) { t.Fatal("precondition: initial acquire should succeed on a free session") } @@ -69,7 +69,7 @@ func TestResumeSession_SkipsWhenBusy(t *testing.T) { // next worker tick) instead of running or marking it. It must return cleanly // without reaching resumeSession's body (nil provider → panic) or markContinued. func TestContinueSession_DefersWhenBusy(t *testing.T) { - a := &agent{gate: newTurnGate()} + a := &agent{gate: turngate.New()} if !a.gate.acquire("sess", 0) { t.Fatal("precondition: initial acquire should succeed on a free session") } diff --git a/cmd/nomos/messagequeue_test.go b/cmd/nomos/messagequeue_test.go deleted file mode 100644 index bec77b72..00000000 --- a/cmd/nomos/messagequeue_test.go +++ /dev/null @@ -1,142 +0,0 @@ -package main - -import ( - "context" - "sync" - "testing" - "time" -) - -func TestMessageQueue_FIFO(t *testing.T) { - q := newMessageQueue() - q.enqueue("s", "first") - q.enqueue("s", "second") - q.enqueue("s", "third") - - want := []string{"first", "second", "third"} - for _, w := range want { - got, ok := q.dequeue("s") - if !ok || got != w { - t.Fatalf("dequeue = %q,%v want %q,true", got, ok, w) - } - } - if _, ok := q.dequeue("s"); ok { - t.Fatal("dequeue on drained queue should return ok=false") - } -} - -func TestMessageQueue_RequeueFront(t *testing.T) { - q := newMessageQueue() - q.enqueue("s", "a") - q.enqueue("s", "b") - // Pop "a", then push it back to the front; "a" must come out before "b". - a, _ := q.dequeue("s") - q.requeueFront("s", a) - got, _ := q.dequeue("s") - if got != "a" { - t.Fatalf("after requeueFront, dequeue = %q want %q", got, "a") - } - got2, _ := q.dequeue("s") - if got2 != "b" { - t.Fatalf("next dequeue = %q want %q", got2, "b") - } -} - -func TestMessageQueue_IsolatedPerSession(t *testing.T) { - q := newMessageQueue() - q.enqueue("s1", "one") - q.enqueue("s2", "two") - if got, _ := q.dequeue("s1"); got != "one" { - t.Fatalf("s1 = %q want one", got) - } - if got, _ := q.dequeue("s2"); got != "two" { - t.Fatalf("s2 = %q want two", got) - } - if q.peek("s1") != 0 || q.peek("s2") != 0 { - t.Fatal("both sessions should be drained") - } -} - -func TestMessageQueue_Concurrent(t *testing.T) { - q := newMessageQueue() - const n = maxQueuedPerSession // stay under the cap so every enqueue lands - var wg sync.WaitGroup - for i := 0; i < n; i++ { - wg.Add(1) - go func(i int) { - defer wg.Done() - q.enqueue("s", "m") - }(i) - } - wg.Wait() - if q.peek("s") != n { - t.Fatalf("peek = %d want %d (all enqueues must be counted)", q.peek("s"), n) - } - seen := 0 - for { - if _, ok := q.dequeue("s"); !ok { - break - } - seen++ - } - if seen != n { - t.Fatalf("drained %d want %d", seen, n) - } -} - -func TestMessageQueue_CapsOverflow(t *testing.T) { - q := newMessageQueue() - for i := 0; i < maxQueuedPerSession; i++ { - if !q.enqueue("s", "m") { - t.Fatalf("enqueue #%d within cap should succeed", i) - } - } - if q.enqueue("s", "overflow") { - t.Fatal("enqueue past the cap should return false (dropped)") - } - if got := q.peek("s"); got != maxQueuedPerSession { - t.Fatalf("peek = %d want %d (overflow must not append)", got, maxQueuedPerSession) - } -} - -// drainQueued on an empty queue must be a no-op: it returns immediately and -// never touches the gate (so the session stays free for the next turn). -func TestDrainQueued_NoOpOnEmpty(t *testing.T) { - a := &agent{gate: newTurnGate(), queue: newMessageQueue()} - a.drainQueued(context.Background(), "s") - if !a.gate.acquire("s", 0) { - t.Fatal("gate should be free after a no-op drain (drain must not hold it)") - } - a.gate.release("s") -} - -// With a queued message but the gate held by another turn, drainQueued must -// re-queue the message and return WITHOUT running a turn (no store/provider → a -// real run would panic). This is the "never stack" property: a busy gate -// defers to the holder's own release-drain. -func TestDrainQueued_RequeuesWhenBusy(t *testing.T) { - prev := drainAcquireWait - drainAcquireWait = 10 * time.Millisecond - t.Cleanup(func() { drainAcquireWait = prev }) - - a := &agent{gate: newTurnGate(), queue: newMessageQueue()} - if !a.gate.acquire("s", 0) { - t.Fatal("precondition: hold the gate") - } - a.queue.enqueue("s", "queued-msg") - - done := make(chan struct{}) - go func() { - a.drainQueued(context.Background(), "s") // must not panic; must requeue - close(done) - }() - select { - case <-done: - case <-time.After(time.Second): - t.Fatal("drainQueued did not return promptly while the gate was busy") - } - if got := a.queue.peek("s"); got != 1 { - t.Fatalf("message should be re-queued while busy; peek = %d want 1", got) - } - a.gate.release("s") -} diff --git a/cmd/nomos/server.go b/cmd/nomos/server.go index 7b596b80..2565fcf8 100644 --- a/cmd/nomos/server.go +++ b/cmd/nomos/server.go @@ -14,9 +14,9 @@ import ( "syscall" "time" + "github.com/dtoro/oikos/internal/nomos/session" "github.com/dtoro/oikos/internal/safego" "github.com/dtoro/oikos/internal/secrets" - "github.com/jackc/pgx/v5" ) func main() { @@ -90,13 +90,13 @@ func main() { probe.close() } - st, err := newStore(ctx, databaseURL) + st, err := session.New(ctx, databaseURL) if err != nil { slog.Error("nomos: db connect", "error", err) os.Exit(1) } if st != nil { - defer st.close() + defer st.Close() } nAgent, err := newAgent(ctx, clientPool, st, agentSlug, openrouterAPIKey) @@ -138,7 +138,7 @@ func main() { case <-ctx.Done(): return case <-ticker.C: - st.cleanupStaleExecutions(ctx, 10*time.Minute) + st.CleanupStaleExecutions(ctx, 10*time.Minute) } } }) @@ -211,7 +211,7 @@ func sseEvent(w http.ResponseWriter, flusher http.Flusher, event agentEvent) { flusher.Flush() } -func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { +func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *session.Store) { if r.Method != http.MethodPost { http.Error(w, "method not allowed", 405) return @@ -238,7 +238,7 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { // abandoned): there's nothing to resume, and running a "report state" // turn there is just a spare turn the operator never asked for (P2.1). if req.Message == "" && req.SessionID != "" { - if sess, err := st.getSession(context.Background(), req.SessionID); err == nil { + if sess, err := st.GetSession(context.Background(), req.SessionID); err == nil { switch sess.Status { case "done", "failed", "abandoned": slog.Info("nomos: reconnect skipped — session already terminal", "session", req.SessionID, "status", sess.Status) @@ -249,7 +249,7 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { slog.Info("nomos: reconnect", "session", req.SessionID) safego.Go("nomos:reconnect:"+req.SessionID, func() { base := "[System: the operator's connection was re-established. The task may have progressed in the background.]" - note := st.enrichResumeNote(context.Background(), req.SessionID, base) + note := st.EnrichResumeNote(context.Background(), req.SessionID, base) a.resumeSession(context.Background(), req.SessionID, note) }) // Return 202 so the frontend doesn't try to consume an SSE stream @@ -299,7 +299,7 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { if sessionID == "" { title := truncate(req.Message, 80) - sess, err := st.createSession(pctx, title) + sess, err := st.CreateSession(pctx, title) if err != nil { slog.Error("nomos: create session", "error", err) sessionID = "ephemeral" @@ -313,24 +313,24 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { // execute). reopenSession marks the prior plan's steps as // `replaced` (proposePlan ignores those) and clears outcome/ // summary. Without this, propose_plan refuses the follow-up with - // errPlanInFlight because the prior steps are all `done`. If the + // ErrPlanInFlight because the prior steps are all `done`. If the // session is still active, reopen is a no-op — the follow-up is // just a continuation of in-flight work. - st.reopenSession(pctx, sessionID) - st.touchSession(pctx, sessionID) + st.ReopenSession(pctx, sessionID) + st.TouchSession(pctx, sessionID) } slog.Info("nomos: chat", "session", sessionID, "message", truncate(req.Message, 100)) userMsg, _ := json.Marshal(map[string]any{"role": "user", "text": req.Message}) - st.saveMessage(pctx, sessionID, "user", userMsg) + st.SaveMessage(pctx, sessionID, "user", userMsg) // If this task has a pending operator question, the incoming message IS the // answer — close it so the panel clears. No separate resume needed: this // chat turn is the resume, and the agent sees the question + answer in its // replayed history. - if qid := st.openQuestionID(pctx, sessionID); qid != "" { - st.answerQuestion(pctx, sessionID, qid, req.Message) + if qid := st.OpenQuestionID(pctx, sessionID); qid != "" { + st.AnswerQuestion(pctx, sessionID, qid, req.Message) } writeEvent(agentEvent{Type: "session", Data: sessionID, SessionID: sessionID}) @@ -343,8 +343,8 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { // real turn server-side. This never stacks concurrent turns — the gate still // guarantees one in-flight turn per session. const turnWait = 5 * time.Second - if !a.gate.acquire(sessionID, turnWait) { - a.queue.enqueue(sessionID, req.Message) + if !a.gate.Acquire(sessionID, turnWait) { + a.queue.Enqueue(sessionID, req.Message) slog.Info("nomos: turn already active, queued operator message", "session", sessionID) writeEvent(agentEvent{Type: "queued", Data: sessionID, SessionID: sessionID}) writeEvent(agentEvent{Type: "done", Data: map[string]any{ @@ -354,7 +354,7 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { return } defer func() { - a.gate.release(sessionID) + a.gate.Release(sessionID) // Run any message that was queued while this turn held the gate. In a // goroutine so the HTTP response finishes without waiting on the next // turn; the queued turn has no SSE client of its own. @@ -394,7 +394,7 @@ func handleChat(w http.ResponseWriter, r *http.Request, a *agent, st *store) { }) } -func handleSessionsList(w http.ResponseWriter, r *http.Request, st *store) { +func handleSessionsList(w http.ResponseWriter, r *http.Request, st *session.Store) { if st == nil { w.Header().Set("Content-Type", "application/json") json.NewEncoder(w).Encode(map[string]any{"sessions": []any{}}) @@ -425,7 +425,7 @@ func handleSessionsList(w http.ResponseWriter, r *http.Request, st *store) { limit = n } } - sessions, err := st.listSessionsFiltered(r.Context(), listFilter{ + sessions, err := st.ListSessionsFiltered(r.Context(), ListFilter{ Outcome: q.Get("outcome"), Status: q.Get("status"), EntityID: q.Get("entity_id"), @@ -457,7 +457,7 @@ func handleSessionsList(w http.ResponseWriter, r *http.Request, st *store) { }) } -func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *agent) { +func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *session.Store, a *agent) { if st == nil { http.Error(w, "not found", 404) return @@ -485,7 +485,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a // POST /sessions/{id}/resume — the operator asks the agent to continue. if len(parts) == 2 && parts[1] == "resume" && r.Method == http.MethodPost { base := "[System: the operator wants you to continue. Pick up where you left off — execute the next step of the plan, diagnose and fix any failures, or report progress if everything is done.]" - note := st.enrichResumeNote(context.Background(), id, base) + note := st.EnrichResumeNote(context.Background(), id, base) safego.Go("nomos:resume-session", func() { a.resumeSession(context.Background(), id, note) }) w.WriteHeader(202) return @@ -503,7 +503,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a switch parts[1] { case "plan": all := r.URL.Query().Has("all") && r.URL.Query().Get("all") != "0" && r.URL.Query().Get("all") != "false" - steps, err := st.getPlanSteps(r.Context(), id, all) + steps, err := st.GetPlanSteps(r.Context(), id, all) if err != nil { http.Error(w, err.Error(), 500) return @@ -512,7 +512,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a json.NewEncoder(w).Encode(map[string]any{"steps": steps}) return case "questions": - questions, err := st.getQuestions(r.Context(), id) + questions, err := st.GetQuestions(r.Context(), id) if err != nil { http.Error(w, err.Error(), 500) return @@ -521,7 +521,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a json.NewEncoder(w).Encode(map[string]any{"questions": questions}) return case "tool_calls": - calls, err := st.getSessionToolCalls(r.Context(), id) + calls, err := st.GetSessionToolCalls(r.Context(), id) if err != nil { http.Error(w, err.Error(), 500) return @@ -534,7 +534,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a switch r.Method { case http.MethodDelete: - if err := st.deleteSession(r.Context(), id); err != nil { + if err := st.DeleteSession(r.Context(), id); err != nil { http.Error(w, err.Error(), 500) return } @@ -551,7 +551,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a // pending_approvals, message_count, tool_call_count, etc. The // messages field is unchanged. Clients that only read // `messages` keep working. - sess, err := st.getSession(r.Context(), id) + sess, err := st.GetSession(r.Context(), id) if err != nil { if err == pgx.ErrNoRows { http.Error(w, "session not found", 404) @@ -560,7 +560,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a http.Error(w, err.Error(), 500) return } - messages, err := st.getMessages(r.Context(), id) + messages, err := st.GetMessages(r.Context(), id) if err != nil { http.Error(w, err.Error(), 500) return @@ -580,7 +580,7 @@ func handleSessionDetail(w http.ResponseWriter, r *http.Request, st *store, a *a // handleAnswerQuestion records the operator's answer to a pinned question and // resumes the agent in the background with that answer injected. Returns 202 — // the agent's response lands via the normal message-polling path, not this POST. -func handleAnswerQuestion(w http.ResponseWriter, r *http.Request, st *store, a *agent, sessionID, questionID string) { +func handleAnswerQuestion(w http.ResponseWriter, r *http.Request, st *session.Store, a *agent, sessionID, questionID string) { var req struct { Answer string `json:"answer"` } @@ -588,15 +588,15 @@ func handleAnswerQuestion(w http.ResponseWriter, r *http.Request, st *store, a * http.Error(w, "answer is required", 400) return } - prompt, _, _ := st.getQuestion(r.Context(), questionID) - if err := st.answerQuestion(r.Context(), sessionID, questionID, req.Answer); err != nil { + prompt, _, _ := st.GetQuestion(r.Context(), questionID) + if err := st.AnswerQuestion(r.Context(), sessionID, questionID, req.Answer); err != nil { http.Error(w, err.Error(), 500) return } if a != nil { base := fmt.Sprintf("[System: the operator answered your question %q with: %q. "+ "Continue the task from here — do not re-ask.]", prompt, req.Answer) - note := st.enrichResumeNote(context.Background(), sessionID, base) + note := st.EnrichResumeNote(context.Background(), sessionID, base) safego.Go("nomos:resume-session", func() { a.resumeSession(context.Background(), sessionID, note) }) } w.WriteHeader(202) diff --git a/cmd/nomos/tasks.go b/cmd/nomos/tasks.go index 1496814e..f33bccec 100644 --- a/cmd/nomos/tasks.go +++ b/cmd/nomos/tasks.go @@ -175,7 +175,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args if strings.TrimSpace(goal) == "" { return "error: set_goal needs a goal", true } - if err := a.store.setGoal(ctx, sessionID, goal); err != nil { + if err := a.store.SetGoal(ctx, sessionID, goal); err != nil { return fmt.Sprintf("error setting goal: %v", err), true } // P1: the plan window is NOT opened here. Opening it on set_goal @@ -196,7 +196,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args // goal + summary; if it looks related, search_knowledge or open // the prior session's transcript (GET /sessions/{id}) before // re-planning. See plans/2026-07-20-session-review-ten-sessions.md. - prior, _ := a.store.recentPartialSessions(ctx, sessionID, 24*time.Hour) + prior, _ := a.store.RecentPartialSessions(ctx, sessionID, 24*time.Hour) if len(prior) > 0 { var b strings.Builder b.WriteString("\n\nNOTE — recent unfinished sessions (last 24h, outcome=partial/failed):") @@ -222,7 +222,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args case "propose_plan": raw, _ := args["steps"].([]any) - var steps []planStepInput + var steps []PlanStepInput for _, r := range raw { m, ok := r.(map[string]any) if !ok { @@ -234,7 +234,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args } detail, _ := m["detail"].(string) target, _ := m["target_slug"].(string) - steps = append(steps, planStepInput{Title: title, Detail: detail, TargetSlug: target}) + steps = append(steps, PlanStepInput{Title: title, Detail: detail, TargetSlug: target}) } if len(steps) == 0 { return "error: propose_plan needs at least one step with a title", true @@ -263,15 +263,15 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args } appendedNote := "" if !hasWritebackStep { - steps = append(steps, planStepInput{ + 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) + persisted, err := a.store.ProposePlan(ctx, sessionID, steps) if err != nil { - if errors.Is(err, errPlanInFlight) { + if errors.Is(err, session.ErrPlanInFlight) { // The plan is already in flight — refuse the re-proposal. // The agent must advance the existing plan with // update_plan_step + run. This is the structural fix for @@ -308,8 +308,8 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args return "error: update_plan_step needs seq (>=1) and status", true } reason, _ := args["replaced_reason"].(string) - if err := a.store.updatePlanStep(ctx, sessionID, seq, status, execID, reason); err != nil { - if errors.Is(err, errPlanStepNotFound) { + if err := a.store.UpdatePlanStep(ctx, sessionID, seq, status, execID, reason); err != nil { + if errors.Is(err, session.ErrPlanStepNotFound) { // The seq doesn't address a step in the CURRENT plan — most // often a stale 1-based number the model carried across a // re-plan, or an out-of-range seq. seq is generation-relative @@ -337,7 +337,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args if ents := toStringSlice(args["context_entities"]); len(ents) > 0 { qctx["entities"] = ents } - if _, err := a.store.askOperator(ctx, sessionID, prompt, qctx); err != nil { + if _, err := a.store.AskOperator(ctx, sessionID, prompt, qctx); err != nil { return fmt.Sprintf("error posting question: %v", err), true } return "Question posted to the operator; the task is paused until they answer. " + @@ -372,7 +372,7 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args // 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) { + 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 } // D.2: refuse success when the goal mentions a reachability/uptime @@ -381,20 +381,20 @@ func (a *agent) handleTaskTool(ctx context.Context, sessionID, name string, args // return 200 for a terminal page (ttyd) or fallback while the actual // dashboard is still down. Must call ping_service or run a successful // curl before claiming success. - if outcome == "success" && a.store.hadDiscovery(ctx, sessionID) { - goal := a.store.sessionGoal(ctx, sessionID) - if mentionsReachability(goal) && !a.store.hadRecentVerification(ctx, sessionID) { + if outcome == "success" && a.store.HadDiscovery(ctx, sessionID) { + goal := a.store.SessionGoal(ctx, sessionID) + if mentionsReachability(goal) && !a.store.HadRecentVerification(ctx, sessionID) { return "Refused: the goal involves a reachability or uptime check (\"make X reachable\", \"get X up\", etc.), but no ping_service call or successful curl/HTTP request against the target was detected. Caddy can return 200 for a terminal or fallback page while the actual service is still down — you must verify the service itself, not just the proxy. Call ping_service(target) or run a curl against the actual service URL, then call complete_task again. Outcome held until verified.", true } } - if err := a.store.completeTask(ctx, sessionID, outcome, summary); err != nil { - if errors.Is(err, errTaskAlreadyComplete) { + if err := a.store.CompleteTask(ctx, sessionID, outcome, summary); err != nil { + if errors.Is(err, session.ErrTaskAlreadyComplete) { return "Task is already complete. Do not call complete_task again. If the operator pointed out a UI/sidebar inconsistency, fix it with update_plan_step (reconcile step states) or summarize the panel in your reply — do not re-execute the work.", true } return fmt.Sprintf("error completing task: %v", err), true } result := fmt.Sprintf("Task marked %s: %s", outcome, summary) - if !a.store.hadEntityWriteback(ctx, sessionID) { + if !a.store.HadEntityWriteback(ctx, sessionID) { result += "\n\n⚠️ No entity attributes or relationships were updated in this session. Call update_entity_attributes and create_relationship to persist what you learned about entities before the next session starts from scratch." } return result, true @@ -445,7 +445,7 @@ func (a *agent) autoCompleteTrivialTask(ctx context.Context, sessionID, response if summary == "" { summary = "Answered without further action needed." } - if err := a.store.completeTask(ctx, sessionID, "success", summary); err != nil { + if err := a.store.CompleteTask(ctx, sessionID, "success", summary); err != nil { slog.Error("nomos: auto-complete trivial task failed", "session", sessionID, "error", err) } } @@ -464,7 +464,7 @@ func (a *agent) autoCompleteIfPlanDone(ctx context.Context, sessionID, responseT if a.store == nil || sessionID == "" || sessionID == "ephemeral" { return } - sess, err := a.store.getSession(ctx, sessionID) + sess, err := a.store.GetSession(ctx, sessionID) if err != nil || sess.Status != "executing" { return } @@ -474,13 +474,13 @@ func (a *agent) autoCompleteIfPlanDone(ctx context.Context, sessionID, responseT // dead task. Confirmed in eval: agent hits P5 approval gate, turn // ends, auto-complete fires incorrectly because the approval-queue // `run` responses were logged as success=true in agent_activity. - if a.store.hasPendingApprovals(ctx, sessionID) { + if a.store.HasPendingApprovals(ctx, sessionID) { return } - discovery := a.store.hadDiscovery(ctx, sessionID) - writeback := a.store.hadEntityWriteback(ctx, sessionID) + discovery := a.store.HadDiscovery(ctx, sessionID) + writeback := a.store.HadEntityWriteback(ctx, sessionID) // (a) all plan steps terminal, OR (b) agent did discovery (ran `run`). - shouldComplete := a.store.allPlanStepsTerminal(ctx, sessionID) + shouldComplete := a.store.AllPlanStepsTerminal(ctx, sessionID) if !shouldComplete && discovery { shouldComplete = true } @@ -500,7 +500,7 @@ func (a *agent) autoCompleteIfPlanDone(ctx context.Context, sessionID, responseT if summary == "" { summary = "All plan steps completed." } - if err := a.store.completeTask(ctx, sessionID, outcome, summary); err != nil { + if err := a.store.CompleteTask(ctx, sessionID, outcome, summary); err != nil { slog.Error("nomos: auto-complete plan-done task failed", "session", sessionID, "error", err) } else { slog.Info("nomos: auto-completed task — agent didn't call complete_task", "session", sessionID, "outcome", outcome) diff --git a/cmd/nomos/workers.go b/cmd/nomos/workers.go index d8a7a023..8a91d316 100644 --- a/cmd/nomos/workers.go +++ b/cmd/nomos/workers.go @@ -28,7 +28,7 @@ func (a *agent) runChatTurn(pctx, ctx context.Context, sessionID, message string var finalThinking string placeholder, _ := json.Marshal(map[string]any{"role": "assistant", "text": ""}) - msgID, err := a.store.insertMessageReturningID(pctx, sessionID, "assistant", placeholder) + msgID, err := a.store.InsertMessageReturningID(pctx, sessionID, "assistant", placeholder) if err != nil { slog.Error("nomos: chat placeholder insert failed", "session", sessionID, "error", err) } @@ -42,7 +42,7 @@ func (a *agent) runChatTurn(pctx, ctx context.Context, sessionID, message string "thinking": finalThinking, "tool_calls": toolCalls, }) - a.store.updateMessage(pctx, msgID, body) + a.store.UpdateMessage(pctx, msgID, body) } a.chat(ctx, sessionID, message, func(ev agentEvent) { @@ -86,7 +86,7 @@ func (a *agent) runChatTurn(pctx, ctx context.Context, sessionID, message string // empty-response'd and all retries failed), delete the placeholder row // instead of persisting an empty bubble. if finalText == "" && len(toolCalls) == 0 && msgID != uuid.Nil { - a.store.deleteMessage(pctx, msgID) + a.store.DeleteMessage(pctx, msgID) } else { persist() // final state — same row, updated one last time } @@ -94,7 +94,7 @@ func (a *agent) runChatTurn(pctx, ctx context.Context, sessionID, message string // Title: prefer the goal once set; else the first assistant answer. if finalText != "" && sessionID != "ephemeral" { var goalTitle string - if sess, gerr := a.store.getSession(pctx, sessionID); gerr == nil && sess.Goal != "" { + if sess, gerr := a.store.GetSession(pctx, sessionID); gerr == nil && sess.Goal != "" { goalTitle = truncate(sess.Goal, 120) } title := goalTitle @@ -102,7 +102,7 @@ func (a *agent) runChatTurn(pctx, ctx context.Context, sessionID, message string title = truncate(finalText, 80) } if title != "" { - a.store.updateSessionTitle(pctx, sessionID, title) + a.store.UpdateSessionTitle(pctx, sessionID, title) } } } @@ -125,14 +125,14 @@ var drainAcquireWait = 5 * time.Second // result via the 3s poller and the status-driven "working" indicator. func (a *agent) drainQueued(ctx context.Context, sessionID string) { for { - msg, ok := a.queue.dequeue(sessionID) + msg, ok := a.queue.Dequeue(sessionID) if !ok { return } // Block briefly for the gate. If a live turn grabbed it first, put the // message back — that turn's release will drain it again. Never stack. - if !a.gate.acquire(sessionID, drainAcquireWait) { - a.queue.requeueFront(sessionID, msg) + if !a.gate.Acquire(sessionID, drainAcquireWait) { + a.queue.RequeueFront(sessionID, msg) return } slog.Info("nomos: running queued operator message", "session", sessionID) @@ -145,7 +145,7 @@ func (a *agent) drainQueued(ctx context.Context, sessionID string) { // the loop would be wrong too: Go defers run at function exit, not // iteration exit, so the gate would stay held across iterations. func() { - defer a.gate.release(sessionID) + defer a.gate.Release(sessionID) a.runChatTurn(pctx, ctx, sessionID, msg, func(agentEvent) {}) }() } diff --git a/internal/adapters/postgres/pool.go b/internal/adapters/postgres/pool.go index 81023e2e..86a6f79f 100644 --- a/internal/adapters/postgres/pool.go +++ b/internal/adapters/postgres/pool.go @@ -5,12 +5,9 @@ import ( "crypto/sha256" "encoding/hex" "fmt" - "io/fs" "log/slog" - "sort" - "strings" - "github.com/dtoro/oikos/migrations" + "github.com/dtoro/oikos/internal/migrate" "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "gopkg.in/yaml.v3" @@ -39,99 +36,11 @@ func New(ctx context.Context, databaseURL string) (*Pool, error) { return &Pool{pool}, nil } -// migrationLockKey is the advisory-lock key serializing migration runs — -// two concurrent `oikos migrate` invocations must not interleave DDL. -const migrationLockKey = 0x01c05e5 - -// Migrate runs all embedded forward migrations in order. -// Uses a schema_migrations table to track applied versions. The whole run -// happens on one connection holding a session advisory lock. +// Migrate applies all embedded forward migrations in order (delegates to +// the shared runner in internal/migrate — the same one nomos's session +// tests use; ADR 0016 rule 3 keeps nomos off the adapters). func (p *Pool) Migrate(ctx context.Context) error { - conn, err := p.Acquire(ctx) - if err != nil { - return fmt.Errorf("acquire migration conn: %w", err) - } - defer conn.Release() - - if _, err := conn.Exec(ctx, "SELECT pg_advisory_lock($1)", migrationLockKey); err != nil { - return fmt.Errorf("acquire migration lock: %w", err) - } - defer func() { - if _, err := conn.Exec(context.WithoutCancel(ctx), "SELECT pg_advisory_unlock($1)", migrationLockKey); err != nil { - slog.Warn("postgres: release migration lock failed", "error", err) - } - }() - - // Create tracking table if not exists - _, err = conn.Exec(ctx, ` - CREATE TABLE IF NOT EXISTS schema_migrations ( - version INT PRIMARY KEY, - applied_at TIMESTAMPTZ NOT NULL DEFAULT now() - ) - `) - if err != nil { - return fmt.Errorf("create schema_migrations: %w", err) - } - - // List migration files - entries, err := fs.ReadDir(migrations.FS, ".") - if err != nil { - return fmt.Errorf("read migration fs: %w", err) - } - - var files []string - for _, e := range entries { - if !e.IsDir() && hasSuffix(e.Name(), ".up.sql") { - files = append(files, e.Name()) - } - } - sort.Strings(files) - - for _, fname := range files { - // Extract version number (001, 002, etc.) - var version int - if _, err := fmt.Sscanf(fname, "%03d", &version); err != nil { - continue - } - - // Check if already applied - var applied int - err := conn.QueryRow(ctx, - "SELECT COUNT(*) FROM schema_migrations WHERE version = $1", version).Scan(&applied) - if err != nil { - return fmt.Errorf("check migration %d: %w", version, err) - } - if applied > 0 { - continue - } - - // Read and execute migration — split into individual statements - // because TimescaleDB CAGGs and some DDL can't run inside a transaction, - // and pgx's multi-statement Exec wraps them implicitly. - content, err := migrations.FS.ReadFile(fname) - if err != nil { - return fmt.Errorf("read %s: %w", fname, err) - } - - stmts := splitSQL(string(content)) - for i, stmt := range stmts { - stmt = strings.TrimSpace(stmt) - if stmt == "" { - continue - } - _, err := conn.Exec(ctx, stmt) - if err != nil { - return fmt.Errorf("exec migration %s stmt %d: %w", fname, i+1, err) - } - } - _, err = conn.Exec(ctx, "INSERT INTO schema_migrations (version) VALUES ($1)", version) - if err != nil { - return fmt.Errorf("record migration %d: %w", version, err) - } - slog.Info("migration applied", "file", fname, "version", version, "statements", len(stmts)) - } - - return nil + return migrate.Apply(ctx, p.Pool) } // SeedIngest ingests a YAML seed file into the database. @@ -191,98 +100,3 @@ func contentHash(content []byte) string { h := sha256.Sum256(content) return hex.EncodeToString(h[:]) } - -// hasSuffix reports whether the string ends with the given suffix. -func hasSuffix(s, suffix string) bool { - return strings.HasSuffix(s, suffix) -} - -// splitSQL splits a SQL string into individual statements. -// Handles $$ ... $$ dollar-quoted blocks, $tag$ ... $tag$ tagged quotes, -// -- line comments, /* ... */ block comments, and '...' string literals -// so that semicolons inside any of these constructs are not treated as -// statement boundaries. -func splitSQL(sql string) []string { - var statements []string - var current strings.Builder - inDollarQuote := false - dollarTag := "" - - i := 0 - for i < len(sql) { - // Handle line comments (-- to end of line) - if !inDollarQuote && i+1 < len(sql) && sql[i] == '-' && sql[i+1] == '-' { - for i < len(sql) && sql[i] != '\n' { - current.WriteByte(sql[i]) - i++ - } - continue - } - - // Handle block comments (/* ... */) - if !inDollarQuote && i+1 < len(sql) && sql[i] == '/' && sql[i+1] == '*' { - end := strings.Index(sql[i+2:], "*/") - if end >= 0 { - current.WriteString(sql[i : i+end+4]) - i += end + 4 - continue - } - } - - // Handle single-quoted string literals ('...') - if !inDollarQuote && sql[i] == '\'' { - j := i + 1 - for j < len(sql) { - if sql[j] == '\'' { - if j+1 < len(sql) && sql[j+1] == '\'' { - j += 2 // skip doubled quote '' - continue - } - break - } - j++ - } - current.WriteString(sql[i : j+1]) - i = j + 1 - continue - } - - // Check for dollar-quote start/end - if !inDollarQuote && sql[i] == '$' { - j := i + 1 - for j < len(sql) && (sql[j] == '_' || (sql[j] >= 'a' && sql[j] <= 'z') || (sql[j] >= 'A' && sql[j] <= 'Z') || (sql[j] >= '0' && sql[j] <= '9')) { - j++ - } - if j < len(sql) && sql[j] == '$' { - dollarTag = sql[i : j+1] - current.WriteString(dollarTag) - inDollarQuote = true - i = j + 1 - continue - } - } - if inDollarQuote && strings.HasPrefix(sql[i:], dollarTag) { - current.WriteString(dollarTag) - i += len(dollarTag) - inDollarQuote = false - dollarTag = "" - continue - } - - if !inDollarQuote && sql[i] == ';' { - statements = append(statements, current.String()) - current.Reset() - i++ - continue - } - - current.WriteByte(sql[i]) - i++ - } - - if strings.TrimSpace(current.String()) != "" { - statements = append(statements, current.String()) - } - - return statements -} diff --git a/internal/migrate/migrate.go b/internal/migrate/migrate.go new file mode 100644 index 00000000..a3f502de --- /dev/null +++ b/internal/migrate/migrate.go @@ -0,0 +1,119 @@ +// Package migrate applies the embedded forward-only SQL migrations +// (migrations/*.up.sql). It is shared infrastructure: the oikos postgres +// adapter runs it at pool startup, and nomos's session tests use it to +// build throwaway databases — nomos must not import the adapters, so the +// runner lives here, one level above both (ADR 0016 rule 3). +package migrate + +import ( + "context" + "fmt" + "io/fs" + "log/slog" + "sort" + "strings" + + "github.com/dtoro/oikos/migrations" + "github.com/jackc/pgx/v5/pgxpool" +) + +// migrationLockKey is the advisory-lock key serializing migration runs — +// two concurrent migrators must not interleave DDL. +const migrationLockKey = 0x01c05e5 + +// PgxPool is the surface Apply needs: acquire a dedicated connection for +// the lock-held run. +type PgxPool interface { + Acquire(ctx context.Context) (*pgxpool.Conn, error) +} + +// Apply runs all embedded forward migrations in order, on one connection +// holding a session advisory lock. Uses the schema_migrations table to +// track applied versions. +func Apply(ctx context.Context, pool PgxPool) error { + conn, err := pool.Acquire(ctx) + if err != nil { + return fmt.Errorf("acquire migration conn: %w", err) + } + defer conn.Release() + + if _, err := conn.Exec(ctx, "SELECT pg_advisory_lock($1)", migrationLockKey); err != nil { + return fmt.Errorf("acquire migration lock: %w", err) + } + defer func() { + if _, err := conn.Exec(context.WithoutCancel(ctx), "SELECT pg_advisory_unlock($1)", migrationLockKey); err != nil { + slog.Warn("migrate: release lock failed", "error", err) + } + }() + + // Create tracking table if not exists + _, err = conn.Exec(ctx, ` + CREATE TABLE IF NOT EXISTS schema_migrations ( + version INT PRIMARY KEY, + applied_at TIMESTAMPTZ NOT NULL DEFAULT now() + ) + `) + if err != nil { + return fmt.Errorf("create schema_migrations: %w", err) + } + + // List migration files + entries, err := fs.ReadDir(migrations.FS, ".") + if err != nil { + return fmt.Errorf("read migration fs: %w", err) + } + + var files []string + for _, e := range entries { + if !e.IsDir() && strings.HasSuffix(e.Name(), ".up.sql") { + files = append(files, e.Name()) + } + } + sort.Strings(files) + + for _, fname := range files { + // Extract version number (001, 002, etc.) + var version int + if _, err := fmt.Sscanf(fname, "%03d", &version); err != nil { + continue + } + + // Check if already applied + var applied int + err := conn.QueryRow(ctx, + "SELECT COUNT(*) FROM schema_migrations WHERE version = $1", version).Scan(&applied) + if err != nil { + return fmt.Errorf("check migration %d: %w", version, err) + } + if applied > 0 { + continue + } + + // Read and execute migration — split into individual statements + // because TimescaleDB CAGGs and some DDL can't run inside a transaction, + // and pgx's multi-statement Exec wraps them implicitly. + content, err := migrations.FS.ReadFile(fname) + if err != nil { + return fmt.Errorf("read %s: %w", fname, err) + } + + stmts := SplitSQL(string(content)) + for i, stmt := range stmts { + stmt = strings.TrimSpace(stmt) + if stmt == "" { + continue + } + _, err := conn.Exec(ctx, stmt) + if err != nil { + return fmt.Errorf("exec migration %s stmt %d: %w", fname, i+1, err) + } + } + _, err = conn.Exec(ctx, "INSERT INTO schema_migrations (version) VALUES ($1)", version) + if err != nil { + return fmt.Errorf("record migration %d: %w", version, err) + } + slog.Info("migration applied", "file", fname, "version", version, "statements", len(stmts)) + } + + return nil +} diff --git a/internal/migrate/split.go b/internal/migrate/split.go new file mode 100644 index 00000000..ffe9e86c --- /dev/null +++ b/internal/migrate/split.go @@ -0,0 +1,93 @@ +package migrate + +import "strings" + +// splitSQL splits a SQL string into individual statements. +// Handles $$ ... $$ dollar-quoted blocks, $tag$ ... $tag$ tagged quotes, +// -- line comments, /* ... */ block comments, and '...' string literals +// so that semicolons inside any of these constructs are not treated as +// statement boundaries. +func SplitSQL(sql string) []string { + var statements []string + var current strings.Builder + inDollarQuote := false + dollarTag := "" + + i := 0 + for i < len(sql) { + // Handle line comments (-- to end of line) + if !inDollarQuote && i+1 < len(sql) && sql[i] == '-' && sql[i+1] == '-' { + for i < len(sql) && sql[i] != '\n' { + current.WriteByte(sql[i]) + i++ + } + continue + } + + // Handle block comments (/* ... */) + if !inDollarQuote && i+1 < len(sql) && sql[i] == '/' && sql[i+1] == '*' { + end := strings.Index(sql[i+2:], "*/") + if end >= 0 { + current.WriteString(sql[i : i+end+4]) + i += end + 4 + continue + } + } + + // Handle single-quoted string literals ('...') + if !inDollarQuote && sql[i] == '\'' { + j := i + 1 + for j < len(sql) { + if sql[j] == '\'' { + if j+1 < len(sql) && sql[j+1] == '\'' { + j += 2 // skip doubled quote '' + continue + } + break + } + j++ + } + current.WriteString(sql[i : j+1]) + i = j + 1 + continue + } + + // Check for dollar-quote start/end + if !inDollarQuote && sql[i] == '$' { + j := i + 1 + for j < len(sql) && (sql[j] == '_' || (sql[j] >= 'a' && sql[j] <= 'z') || (sql[j] >= 'A' && sql[j] <= 'Z') || (sql[j] >= '0' && sql[j] <= '9')) { + j++ + } + if j < len(sql) && sql[j] == '$' { + dollarTag = sql[i : j+1] + current.WriteString(dollarTag) + inDollarQuote = true + i = j + 1 + continue + } + } + if inDollarQuote && strings.HasPrefix(sql[i:], dollarTag) { + current.WriteString(dollarTag) + i += len(dollarTag) + inDollarQuote = false + dollarTag = "" + continue + } + + if !inDollarQuote && sql[i] == ';' { + statements = append(statements, current.String()) + current.Reset() + i++ + continue + } + + current.WriteByte(sql[i]) + i++ + } + + if strings.TrimSpace(current.String()) != "" { + statements = append(statements, current.String()) + } + + return statements +} diff --git a/internal/adapters/postgres/splitsql_test.go b/internal/migrate/split_test.go similarity index 84% rename from internal/adapters/postgres/splitsql_test.go rename to internal/migrate/split_test.go index 17c70cde..70e3cf7e 100644 --- a/internal/adapters/postgres/splitsql_test.go +++ b/internal/migrate/split_test.go @@ -1,4 +1,4 @@ -package db +package migrate import ( "strings" @@ -16,7 +16,7 @@ func nonEmpty(stmts []string) []string { } func TestSplitSQLBasic(t *testing.T) { - stmts := nonEmpty(splitSQL("CREATE TABLE a (id int); CREATE TABLE b (id int);")) + stmts := nonEmpty(SplitSQL("CREATE TABLE a (id int); CREATE TABLE b (id int);")) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -27,7 +27,7 @@ func TestSplitSQLDollarQuotedFunction(t *testing.T) { SELECT 1; SELECT 2; $$ LANGUAGE sql; CREATE TABLE t (id int);` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -38,7 +38,7 @@ CREATE TABLE t (id int);` func TestSplitSQLTaggedDollarQuote(t *testing.T) { sql := `DO $body$ BEGIN PERFORM 1; END $body$;SELECT 1;` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -46,7 +46,7 @@ func TestSplitSQLTaggedDollarQuote(t *testing.T) { func TestSplitSQLSemicolonInComment(t *testing.T) { sql := "-- comment with ; semicolon\nCREATE TABLE t (id int); -- trailing; note\nSELECT 1;" - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -54,7 +54,7 @@ func TestSplitSQLSemicolonInComment(t *testing.T) { func TestSplitSQLSemicolonInStringLiteral(t *testing.T) { sql := `SELECT 'hello; world'; INSERT INTO t VALUES (1);` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -62,7 +62,7 @@ func TestSplitSQLSemicolonInStringLiteral(t *testing.T) { func TestSplitSQLDollarSignInStringLiteral(t *testing.T) { sql := `SELECT '$100'; SELECT 2;` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -70,7 +70,7 @@ func TestSplitSQLDollarSignInStringLiteral(t *testing.T) { func TestSplitSQLBlockComment(t *testing.T) { sql := `SELECT 1; /* block; with; semicolons */ SELECT 2;` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } @@ -78,7 +78,7 @@ func TestSplitSQLBlockComment(t *testing.T) { func TestSplitSQLBlockCommentWithDollarQuote(t *testing.T) { sql := `/* $$ not a dollar quote */ SELECT 1;` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 1 { t.Fatalf("got %d statements, want 1: %#v", len(stmts), stmts) } @@ -86,21 +86,21 @@ func TestSplitSQLBlockCommentWithDollarQuote(t *testing.T) { func TestSplitSQLDoubledQuoteInString(t *testing.T) { sql := `SELECT 'O''Brien'; SELECT 2;` - stmts := nonEmpty(splitSQL(sql)) + stmts := nonEmpty(SplitSQL(sql)) if len(stmts) != 2 { t.Fatalf("got %d statements, want 2: %#v", len(stmts), stmts) } } func TestSplitSQLEmptyInput(t *testing.T) { - stmts := nonEmpty(splitSQL("")) + stmts := nonEmpty(SplitSQL("")) if len(stmts) != 0 { t.Fatalf("got %d statements, want 0", len(stmts)) } } func TestSplitSQLNoSemicolon(t *testing.T) { - stmts := nonEmpty(splitSQL("SELECT 1")) + stmts := nonEmpty(SplitSQL("SELECT 1")) if len(stmts) != 1 { t.Fatalf("got %d statements, want 1", len(stmts)) } diff --git a/cmd/nomos/assent.go b/internal/nomos/assent/assent.go similarity index 73% rename from cmd/nomos/assent.go rename to internal/nomos/assent/assent.go index 228e1eb9..a0e1f118 100644 --- a/cmd/nomos/assent.go +++ b/internal/nomos/assent/assent.go @@ -1,11 +1,6 @@ -package main +package assent import ( - "bytes" - "context" - "encoding/json" - "fmt" - "net/http" "regexp" "strings" ) @@ -19,11 +14,11 @@ import ( // something new. Destructive-risk actions are excluded: they always need the // explicit typed-confirmation flow, never loose assent. -// pendingApproval is one gated action proposed in the immediately-preceding +// PendingApproval is one gated action proposed in the immediately-preceding // assistant turn, extracted from its tool_result text. -type pendingApproval struct { - execID string - destructive bool +type PendingApproval struct { + ExecID string + Destructive bool } // executionQueuedRE matches the "execution queued" phrasing shared by @@ -32,17 +27,16 @@ var executionQueuedRE = regexp.MustCompile(`(?i)execution\s+([0-9a-f]{8}-[0-9a-f // extractPendingApprovals scans the tool results of one assistant turn for // gated actions that are still awaiting a decision. -func extractPendingApprovals(calls []persistedCall) []pendingApproval { - var out []pendingApproval - for _, c := range calls { - text := c.resultText() +func ExtractPendingApprovals(resultTexts []string) []PendingApproval { + var out []PendingApproval + for _, text := range resultTexts { m := executionQueuedRE.FindStringSubmatch(text) if m == nil { continue } - out = append(out, pendingApproval{ - execID: m[1], - destructive: strings.Contains(strings.ToUpper(text), "DESTRUCTIVE"), + out = append(out, PendingApproval{ + ExecID: m[1], + Destructive: strings.Contains(strings.ToUpper(text), "DESTRUCTIVE"), }) } return out @@ -115,7 +109,7 @@ func containsPhrase(tokens []string, phrase string) bool { // pending proposal. Deliberately simple and auditable: a fixed word list, // not a model judgment call, so behavior is predictable and can't be // prompt-injected via the pending action's own content. -func isAssent(msg string) bool { +func IsAssent(msg string) bool { tokens := tokenize(msg) for _, w := range negationWords { if containsPhrase(tokens, w) { @@ -137,7 +131,7 @@ func isAssent(msg string) bool { // this is the typed-confirmation phrase SOUL.md tells the operator to use // ("I confirm destroy 135"). Still negation-aware for the same reason as // isAssent: "don't confirm yet" must not accidentally match. -func isTypedConfirmation(msg string) bool { +func IsTypedConfirmation(msg string) bool { tokens := tokenize(msg) for _, w := range negationWords { if containsPhrase(tokens, w) { @@ -147,37 +141,3 @@ func isTypedConfirmation(msg string) bool { return containsPhrase(tokens, "confirm") || containsPhrase(tokens, "confirmed") } -// approveExecution grants (or denies) a pending execution via the same HTTP -// endpoint the chat UI's Approve button calls, so both paths share one code -// path server-side (executeApprovedAction) and one audit trail. Returns the -// decided status, or an error if the request failed outright (a 4xx for an -// already-decided/expired approval is reported via ok=false, not a hard err, -// since that's an expected race, not a bug). -func (a *agent) approveExecution(ctx context.Context, execID string) (ok bool, status string, err error) { - if a.apiBase == "" { - return false, "", fmt.Errorf("no API base configured") - } - body, _ := json.Marshal(map[string]string{"decision": "approve"}) - req, err := http.NewRequestWithContext(ctx, http.MethodPost, - a.apiBase+"/api/v1/approvals/"+execID+"/decision", bytes.NewReader(body)) - if err != nil { - return false, "", err - } - req.Header.Set("Content-Type", "application/json") - if a.apiToken != "" { - req.Header.Set("Authorization", "Bearer "+a.apiToken) - } - resp, err := a.httpClient.Do(req) - if err != nil { - return false, "", err - } - defer resp.Body.Close() - if resp.StatusCode != http.StatusOK { - return false, "", nil // already decided / expired / not found — not a hard failure - } - var out struct { - Status string `json:"status"` - } - json.NewDecoder(resp.Body).Decode(&out) - return true, out.Status, nil -} diff --git a/cmd/nomos/assent_test.go b/internal/nomos/assent/assent_test.go similarity index 74% rename from cmd/nomos/assent_test.go rename to internal/nomos/assent/assent_test.go index 245fa05d..1bc41140 100644 --- a/cmd/nomos/assent_test.go +++ b/internal/nomos/assent/assent_test.go @@ -1,7 +1,6 @@ -package main +package assent import ( - "encoding/json" "testing" ) @@ -106,31 +105,24 @@ func TestIsTypedConfirmation(t *testing.T) { } func TestExtractPendingApprovals(t *testing.T) { - mkCall := func(text string) persistedCall { - b, _ := json.Marshal(text) - return persistedCall{id: "x", name: "run", result: json.RawMessage(b)} - } - calls := []persistedCall{ - mkCall("run on host:strong requires approval (risk: config_mutation) — execution 019f4930-e22b-7c47-8c6e-715dcd59df19 queued. Present the command..."), - mkCall("some unrelated read-only result, no approval here"), - mkCall("run on lxc:caddy requires approval (risk: destructive) — execution 019f4931-aaaa-7c47-8c6e-715dcd59df20 queued. This is classified DESTRUCTIVE — flag that clearly."), - } - got := extractPendingApprovals(calls) + got := ExtractPendingApprovals([]string{ + "run on host:strong requires approval (risk: config_mutation) - execution 019f4930-e22b-7c47-8c6e-715dcd59df19 queued. Present the command...", + "some unrelated read-only result, no approval here", + "run on lxc:caddy requires approval (risk: destructive) - execution 019f4931-aaaa-7c47-8c6e-715dcd59df20 queued. This is classified DESTRUCTIVE - flag that clearly.", + }) if len(got) != 2 { - t.Fatalf("expected 2 pending approvals, got %d: %+v", len(got), got) + t.Fatalf("expected 2 pending approvals, got %d", len(got)) } - if got[0].execID != "019f4930-e22b-7c47-8c6e-715dcd59df19" || got[0].destructive { - t.Errorf("first approval wrong: %+v", got[0]) + if got[0].ExecID != "019f4930-e22b-7c47-8c6e-715dcd59df19" || got[0].Destructive { + t.Errorf("first approval: ExecID=%s Destructive=%v", got[0].ExecID, got[0].Destructive) } - if got[1].execID != "019f4931-aaaa-7c47-8c6e-715dcd59df20" || !got[1].destructive { + if got[1].ExecID != "019f4931-aaaa-7c47-8c6e-715dcd59df20" || !got[1].Destructive { t.Errorf("second approval should be flagged destructive: %+v", got[1]) } } func TestExtractPendingApprovals_NoneWhenNoneQueued(t *testing.T) { - b, _ := json.Marshal("fleet is healthy, nothing to report") - calls := []persistedCall{{id: "x", result: json.RawMessage(b)}} - if got := extractPendingApprovals(calls); len(got) != 0 { - t.Errorf("expected no pending approvals, got %+v", got) + if got := ExtractPendingApprovals(nil); len(got) != 0 { + t.Errorf("expected 0, got %d", len(got)) } } diff --git a/cmd/nomos/messagequeue.go b/internal/nomos/messagequeue/messagequeue.go similarity index 64% rename from cmd/nomos/messagequeue.go rename to internal/nomos/messagequeue/messagequeue.go index 895798cd..c2db1d3f 100644 --- a/cmd/nomos/messagequeue.go +++ b/internal/nomos/messagequeue/messagequeue.go @@ -1,19 +1,19 @@ -package main +package messagequeue import ( "log/slog" "sync" ) -// maxQueuedPerSession caps a session's queue. A held turn plus unbounded +// MaxQueuedPerSession caps a session's queue. A held turn plus unbounded // enqueues would grow memory without limit; an operator nudging a long // autonomous turn realistically queues only a handful, so a generous cap is -// pure insurance. Overflow drops the newest enqueue and logs (the message is -// already persisted in the DB by handleChat before enqueue, so it isn't lost +// pure insurance. Overflow drops the newest Enqueue and logs (the message is +// already persisted in the DB by handleChat before Enqueue, so it isn't lost // from the transcript — it just won't auto-run). -const maxQueuedPerSession = 20 +const MaxQueuedPerSession = 20 -// messageQueue holds operator messages that arrived while a turn was already +// MessageQueue holds operator messages that arrived while a turn was already // running for a session. Plan 2026-08-03 (F2): instead of rejecting the // operator's message with "Nomos is still finishing a previous step… send it // again", the message is queued and auto-run when the in-flight turn releases @@ -28,32 +28,32 @@ const maxQueuedPerSession = 20 // main.go), so this cannot stack concurrent turns — the exact hazard the gate // itself exists to prevent. Background resumeSession callers never touch this // queue; they keep their non-blocking skip. -type messageQueue struct { +type MessageQueue struct { mu sync.Mutex queue map[string][]string } -func newMessageQueue() *messageQueue { - return &messageQueue{queue: map[string][]string{}} +func New() *MessageQueue { + return &MessageQueue{queue: map[string][]string{}} } -// enqueue appends a message to the back of the session's FIFO. Returns false -// (and logs) if the session is already at maxQueuedPerSession — the caller's +// Enqueue appends a message to the back of the session's FIFO. Returns false +// (and logs) if the session is already at MaxQueuedPerSession — the caller's // message is already persisted in the DB, so this only skips auto-running it. -func (q *messageQueue) enqueue(sessionID, msg string) bool { +func (q *MessageQueue) Enqueue(sessionID, msg string) bool { q.mu.Lock() defer q.mu.Unlock() - if len(q.queue[sessionID]) >= maxQueuedPerSession { - slog.Warn("nomos: message queue full; dropping auto-run for operator message", "session", sessionID, "cap", maxQueuedPerSession) + if len(q.queue[sessionID]) >= MaxQueuedPerSession { + slog.Warn("nomos: message queue full; dropping auto-run for operator message", "session", sessionID, "cap", MaxQueuedPerSession) return false } q.queue[sessionID] = append(q.queue[sessionID], msg) return true } -// dequeue pops the next message from the front of the session's FIFO. Returns +// Dequeue pops the next message from the front of the session's FIFO. Returns // ok=false when empty. -func (q *messageQueue) dequeue(sessionID string) (string, bool) { +func (q *MessageQueue) Dequeue(sessionID string) (string, bool) { q.mu.Lock() defer q.mu.Unlock() xs := q.queue[sessionID] @@ -65,17 +65,17 @@ func (q *messageQueue) dequeue(sessionID string) (string, bool) { return m, true } -// requeueFront pushes a message back to the front — used when a drainer popped +// RequeueFront pushes a message back to the front — used when a drainer popped // a message but lost the race for the gate to a live turn; that turn's own // release will drain it again. -func (q *messageQueue) requeueFront(sessionID, msg string) { +func (q *MessageQueue) RequeueFront(sessionID, msg string) { q.mu.Lock() defer q.mu.Unlock() q.queue[sessionID] = append([]string{msg}, q.queue[sessionID]...) } -// peek reports the queued depth for a session (test/diagnostic helper). -func (q *messageQueue) peek(sessionID string) int { +// Peek reports the queued depth for a session (test/diagnostic helper). +func (q *MessageQueue) Peek(sessionID string) int { q.mu.Lock() defer q.mu.Unlock() return len(q.queue[sessionID]) diff --git a/internal/nomos/messagequeue/messagequeue_test.go b/internal/nomos/messagequeue/messagequeue_test.go new file mode 100644 index 00000000..d570629e --- /dev/null +++ b/internal/nomos/messagequeue/messagequeue_test.go @@ -0,0 +1,98 @@ +package messagequeue + +import ( + "sync" + "testing" +) + +func TestMessageQueue_FIFO(t *testing.T) { + q := New() + q.Enqueue("s", "first") + q.Enqueue("s", "second") + q.Enqueue("s", "third") + + want := []string{"first", "second", "third"} + for _, w := range want { + got, ok := q.Dequeue("s") + if !ok || got != w { + t.Fatalf("Dequeue = %q,%v want %q,true", got, ok, w) + } + } + if _, ok := q.Dequeue("s"); ok { + t.Fatal("Dequeue on drained queue should return ok=false") + } +} + +func TestMessageQueue_RequeueFront(t *testing.T) { + q := New() + q.Enqueue("s", "a") + q.Enqueue("s", "b") + // Pop "a", then push it back to the front; "a" must come out before "b". + a, _ := q.Dequeue("s") + q.RequeueFront("s", a) + got, _ := q.Dequeue("s") + if got != "a" { + t.Fatalf("after RequeueFront, Dequeue = %q want %q", got, "a") + } + got2, _ := q.Dequeue("s") + if got2 != "b" { + t.Fatalf("next Dequeue = %q want %q", got2, "b") + } +} + +func TestMessageQueue_IsolatedPerSession(t *testing.T) { + q := New() + q.Enqueue("s1", "one") + q.Enqueue("s2", "two") + if got, _ := q.Dequeue("s1"); got != "one" { + t.Fatalf("s1 = %q want one", got) + } + if got, _ := q.Dequeue("s2"); got != "two" { + t.Fatalf("s2 = %q want two", got) + } + if q.Peek("s1") != 0 || q.Peek("s2") != 0 { + t.Fatal("both sessions should be drained") + } +} + +func TestMessageQueue_Concurrent(t *testing.T) { + q := New() + const n = MaxQueuedPerSession // stay under the cap so every Enqueue lands + var wg sync.WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + q.Enqueue("s", "m") + }(i) + } + wg.Wait() + if q.Peek("s") != n { + t.Fatalf("Peek = %d want %d (all enqueues must be counted)", q.Peek("s"), n) + } + seen := 0 + for { + if _, ok := q.Dequeue("s"); !ok { + break + } + seen++ + } + if seen != n { + t.Fatalf("drained %d want %d", seen, n) + } +} + +func TestMessageQueue_CapsOverflow(t *testing.T) { + q := New() + for i := 0; i < MaxQueuedPerSession; i++ { + if !q.Enqueue("s", "m") { + t.Fatalf("Enqueue #%d within cap should succeed", i) + } + } + if q.Enqueue("s", "overflow") { + t.Fatal("Enqueue past the cap should return false (dropped)") + } + if got := q.Peek("s"); got != MaxQueuedPerSession { + t.Fatalf("Peek = %d want %d (overflow must not append)", got, MaxQueuedPerSession) + } +} diff --git a/cmd/nomos/retrycap.go b/internal/nomos/retrycap/retrycap.go similarity index 79% rename from cmd/nomos/retrycap.go rename to internal/nomos/retrycap/retrycap.go index 2144f02c..97ce95dc 100644 --- a/cmd/nomos/retrycap.go +++ b/internal/nomos/retrycap/retrycap.go @@ -1,4 +1,4 @@ -package main +package retrycap import ( "crypto/sha256" @@ -7,8 +7,8 @@ import ( "sync" ) -// maxRunRetries is the per-turn cap on identical failing `run` tool calls. -// After this many failures with the same (target, command) key, the agent +// MaxRunRetries is the per-turn cap on identical failing `run` tool calls. +// After this many Failures with the same (target, command) key, the agent // loop refuses to dispatch the call again and instead surfaces a directive // to investigate *why* (ps/strace/lsof) or escalate to the operator. // @@ -20,9 +20,9 @@ import ( // only investigated *why* after the operator explicitly asked // "the command just keeps running?" — see // plans/2026-07-18-session-review-three-sessions.md P0.1. -const maxRunRetries = 3 +const MaxRunRetries = 3 -// runRetryTracker deduplicates failing `run` calls within a single chat +// RunRetryTracker deduplicates failing `run` calls within a single chat // turn (chatWith invocation). It is NOT persisted across turns — the cap // is per-turn, so a fresh turn after the operator responds can retry once // more. The intent is to break a tight retry loop within one turn, not to @@ -31,51 +31,51 @@ const maxRunRetries = 3 // Threading: the agent loop is single-goroutine per turn, but the tracker // is guarded by a mutex so future callers (e.g. concurrent tool dispatch) // stay safe. The mutex is uncontended on the current hot path. -type runRetryTracker struct { +type RunRetryTracker struct { mu sync.Mutex counts map[string]int } -func newRunRetryTracker() *runRetryTracker { - return &runRetryTracker{counts: make(map[string]int)} +func New() *RunRetryTracker { + return &RunRetryTracker{counts: make(map[string]int)} } -// runFailureKey is the dedup key for "this is the same command against the +// RunFailureKey is the dedup key for "this is the same command against the // same target." Whitespace is collapsed so trivial reformatting // (newlines vs spaces, trailing whitespace) doesn't escape the cap. The // purpose field is intentionally NOT part of the key: the agent often // rephrases purpose between retries while issuing the same command. -func runFailureKey(target, command string) string { +func RunFailureKey(target, command string) string { collapsed := strings.Join(strings.Fields(command), " ") target = strings.TrimSpace(target) h := sha256.Sum256([]byte(target + "\x00" + collapsed)) return hex.EncodeToString(h[:]) } -// recordFailure increments the failure count for the given key and returns -// the new count. The caller should check `count > maxRunRetries` BEFORE +// RecordFailure increments the failure count for the given key and returns +// the new count. The caller should check `count > MaxRunRetries` BEFORE // dispatching to decide whether to skip the call. -func (r *runRetryTracker) recordFailure(key string) int { +func (r *RunRetryTracker) RecordFailure(key string) int { r.mu.Lock() defer r.mu.Unlock() r.counts[key]++ return r.counts[key] } -// failures returns the current failure count for a key (0 if unseen). -func (r *runRetryTracker) failures(key string) int { +// Failures returns the current failure count for a key (0 if unseen). +func (r *RunRetryTracker) Failures(key string) int { r.mu.Lock() defer r.mu.Unlock() return r.counts[key] } -// isRunFailure reports whether a `run` tool call's outcome should count +// IsRunFailure reports whether a `run` tool call's outcome should count // as a failure for retry-cap purposes. A call counts as failed when: // - the dispatch itself errored (callErr != nil), OR // - the result text starts with "run on : ERROR" — the // shape classifyAndGate/sshExec produce when SSH or the command fails. // -// Approvals queued ("requires approval") do NOT count as failures: they +// Approvals queued ("requires approval") do NOT count as Failures: they // are pending operator action, not a command execution failure. A read // of the existing code paths (classifyAndGate in internal/mcp/server.go) // confirms the "ERROR" prefix is the stable failure signature for `run`. @@ -85,7 +85,7 @@ func (r *runRetryTracker) failures(key string) int { // "run on host:strong: ERROR ...", the MCP client unwraps it back to a // plain Go string (see mcpClient.callTool). The caller should pass that // raw string, not json.Marshal's output (which would quote-wrap it). -func isRunFailure(toolName string, resultText string, callErr error) bool { +func IsRunFailure(toolName string, resultText string, callErr error) bool { if callErr != nil { return true } @@ -100,12 +100,12 @@ func isRunFailure(toolName string, resultText string, callErr error) bool { return strings.Contains(resultText, ": ERROR") } -// runResultText extracts the raw text from a `run` tool's result value as +// RunResultText extracts the raw text from a `run` tool's result value as // returned by mcpClient.callTool — typically a Go string, but may also be // a []string (multi-content result) or other JSON-decoded shape. Returns // "" for shapes we don't recognize. Used by the retry-cap path so -// isRunFailure receives the un-quoted text form (see its doc comment). -func runResultText(result any) string { +// IsRunFailure receives the un-quoted text form (see its doc comment). +func RunResultText(result any) string { switch v := result.(type) { case string: return v @@ -125,13 +125,13 @@ func runResultText(result any) string { return "" } -// runRetryDirective is the synthetic tool result returned to the model +// RunRetryDirective is the synthetic tool result returned to the model // when the retry cap is hit, in place of dispatching the call again. It // directs the agent to investigate *why* the command keeps failing before // retrying, or to surface the blocker to the operator. -func runRetryDirective(target, command string, failures int) string { +func RunRetryDirective(target, command string, Failures int) string { return "Refused: this `run` against " + target + " has failed " + - itoa(failures) + " times this turn — retry cap hit. The command:\n " + + itoa(Failures) + " times this turn — retry cap hit. The command:\n " + command + "\nis almost certainly blocked by something on the target " + "(a hung process, a kernel lock, an unexported FS, a stuck SSH " + "session, …) — NOT a transient gateway issue. Do NOT retry with " + diff --git a/cmd/nomos/retrycap_test.go b/internal/nomos/retrycap/retrycap_test.go similarity index 68% rename from cmd/nomos/retrycap_test.go rename to internal/nomos/retrycap/retrycap_test.go index 2012e4ba..d4ea4622 100644 --- a/cmd/nomos/retrycap_test.go +++ b/internal/nomos/retrycap/retrycap_test.go @@ -1,4 +1,4 @@ -package main +package retrycap import ( "strings" @@ -15,8 +15,8 @@ func TestRunFailureKey_StableAcrossWhitespace(t *testing.T) { " chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local"}, } for i, c := range cases { - ka := runFailureKey("host:strong", c.a) - kb := runFailureKey("host:strong", c.b) + ka := RunFailureKey("host:strong", c.a) + kb := RunFailureKey("host:strong", c.b) if ka != kb { t.Errorf("case %d: keys differ for whitespace-equivalent commands:\n a=%q\n b=%q", i, c.a, c.b) } @@ -24,49 +24,49 @@ func TestRunFailureKey_StableAcrossWhitespace(t *testing.T) { } func TestRunFailureKey_DiffersByTarget(t *testing.T) { - a := runFailureKey("host:strong", "echo hi") - b := runFailureKey("host:hubris", "echo hi") + a := RunFailureKey("host:strong", "echo hi") + b := RunFailureKey("host:hubris", "echo hi") if a == b { t.Error("keys should differ when target differs") } } func TestRunFailureKey_DiffersByCommand(t *testing.T) { - a := runFailureKey("host:strong", "echo hi") - b := runFailureKey("host:strong", "echo bye") + a := RunFailureKey("host:strong", "echo hi") + b := RunFailureKey("host:strong", "echo bye") if a == b { t.Error("keys should differ when command differs") } } func TestRunRetryTracker_CountsAndCaps(t *testing.T) { - r := newRunRetryTracker() - key := runFailureKey("host:strong", "chown :10000 /mnt/media_local") - for i := 1; i <= maxRunRetries; i++ { - if got := r.recordFailure(key); got != i { - t.Errorf("recordFailure #%d = %d, want %d", i, got, i) + r := New() + key := RunFailureKey("host:strong", "chown :10000 /mnt/media_local") + for i := 1; i <= MaxRunRetries; i++ { + if got := r.RecordFailure(key); got != i { + t.Errorf("RecordFailure #%d = %d, want %d", i, got, i) } } - // At the cap, failures() should report maxRunRetries, and the next - // identical call should be refused by the agent loop (failures() >= - // maxRunRetries). - if got := r.failures(key); got != maxRunRetries { - t.Errorf("failures = %d, want %d", got, maxRunRetries) + // At the cap, Failures() should report MaxRunRetries, and the next + // identical call should be refused by the agent loop (Failures() >= + // MaxRunRetries). + if got := r.Failures(key); got != MaxRunRetries { + t.Errorf("Failures = %d, want %d", got, MaxRunRetries) } - if r.failures(key) < maxRunRetries { - t.Errorf("cap should be enforced at maxRunRetries=%d", maxRunRetries) + if r.Failures(key) < MaxRunRetries { + t.Errorf("cap should be enforced at MaxRunRetries=%d", MaxRunRetries) } } func TestRunRetryTracker_PerTurnIsolation(t *testing.T) { // Different keys don't interfere. - r := newRunRetryTracker() - k1 := runFailureKey("host:strong", "echo a") - k2 := runFailureKey("host:strong", "echo b") - r.recordFailure(k1) - r.recordFailure(k1) - if got := r.failures(k2); got != 0 { - t.Errorf("k2 failures = %d, want 0 (keys are isolated)", got) + r := New() + k1 := RunFailureKey("host:strong", "echo a") + k2 := RunFailureKey("host:strong", "echo b") + r.RecordFailure(k1) + r.RecordFailure(k1) + if got := r.Failures(k2); got != 0 { + t.Errorf("k2 Failures = %d, want 0 (keys are isolated)", got) } } @@ -88,9 +88,9 @@ func TestIsRunFailure(t *testing.T) { {"callErr set on non-run tool", "get_entity", "some result", errFake{}, true}, // callErr trumps name } for i, c := range cases { - got := isRunFailure(c.tool, c.result, c.callErr) + got := IsRunFailure(c.tool, c.result, c.callErr) if got != c.want { - t.Errorf("case %d (%s): isRunFailure = %v, want %v", i, c.desc, got, c.want) + t.Errorf("case %d (%s): IsRunFailure = %v, want %v", i, c.desc, got, c.want) } } } @@ -100,7 +100,7 @@ type errFake struct{} func (errFake) Error() string { return "fake dispatch error" } func TestRunRetryDirective_Content(t *testing.T) { - d := runRetryDirective("host:strong", "chown :10000 /mnt/media_local", 3) + d := RunRetryDirective("host:strong", "chown :10000 /mnt/media_local", 3) for _, want := range []string{ "Refused:", "host:strong", diff --git a/cmd/nomos/store.go b/internal/nomos/session/store.go similarity index 100% rename from cmd/nomos/store.go rename to internal/nomos/session/store.go diff --git a/cmd/nomos/store_test.go b/internal/nomos/session/store_test.go similarity index 96% rename from cmd/nomos/store_test.go rename to internal/nomos/session/store_test.go index 9c0dedda..bdaaf1c3 100644 --- a/cmd/nomos/store_test.go +++ b/internal/nomos/session/store_test.go @@ -23,10 +23,10 @@ import ( ) // newTestStore creates a throwaway, fully-migrated database and returns a -// *store connected to it, cleaned up (including a matching task: +// *Store connected to it, cleaned up (including a matching task: // entity type in the ontology, needed by createTaskEntity/proposePlan tests) // via t.Cleanup. -func newTestStore(t *testing.T) *store { +func newTestStore(t *testing.T) *Store { t.Helper() baseURL := os.Getenv("OIKOS_TEST_DATABASE_URL") if baseURL == "" { @@ -74,7 +74,7 @@ func newTestStore(t *testing.T) *store { t.Fatalf("seed minimal ontology: %v", err) } - return &store{pool: pool.Pool} + return &Store{pool: pool.Pool} } func swapTestDatabase(url, dbName string) string { @@ -180,7 +180,7 @@ func TestProposePlan_RefuseInFlight(t *testing.T) { // 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) } @@ -199,7 +199,7 @@ func TestProposePlan_RefuseInFlight(t *testing.T) { // 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"}}) + _, 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) } @@ -223,10 +223,10 @@ func TestProposePlan_RefuseInFlight(t *testing.T) { 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. @@ -275,14 +275,14 @@ func TestUpdatePlanStep_GenerationRelative(t *testing.T) { 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 { 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) } @@ -337,7 +337,7 @@ func TestCompleteTask_AutoCloseEmitsEvents(t *testing.T) { 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. @@ -372,7 +372,7 @@ func TestCompleteTask_AutoCloseEmitsEvents(t *testing.T) { } } -// TestHadDiscoveryAndWriteback is the store-level proof for D.1 (refuse +// 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 @@ -444,7 +444,7 @@ func TestHadDiscoveryAndWriteback(t *testing.T) { } } -// TestSetGoal_SupersessionEvent is the store-level proof for P1.4 from +// 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 @@ -502,7 +502,7 @@ func TestSetGoal_SupersededEvent(t *testing.T) { // 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 { +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`, diff --git a/cmd/nomos/turngate.go b/internal/nomos/turngate/turngate.go similarity index 76% rename from cmd/nomos/turngate.go rename to internal/nomos/turngate/turngate.go index 313e7d48..b7c5a29a 100644 --- a/cmd/nomos/turngate.go +++ b/internal/nomos/turngate/turngate.go @@ -1,11 +1,11 @@ -package main +package turngate import ( "sync" "time" ) -// turnGate enforces at most one in-flight agent turn per session. +// TurnGate enforces at most one in-flight agent turn per session. // // Why this exists (plan 2026-08-03, F1): handleChat runs a turn in the HTTP // request goroutine, and every "resume" path (the empty-message reconnect, @@ -20,7 +20,7 @@ import ( // Model: one permit (buffered-1 channel seeded with a single token) per // session id. Acquiring consumes the token; releasing puts it back. // - Background/best-effort callers (resumeSession and everything it backs) -// use a non-blocking acquire and SKIP when busy — a duplicate nudge while a +// use a non-blocking Acquire and SKIP when busy — a duplicate nudge while a // turn is already running adds nothing, and the continuation/idle tickers // will retry on their own. // - The live chat path (an operator message) waits briefly for a finishing @@ -29,21 +29,21 @@ import ( // // The permits map grows one entry per session id seen. For this single-agent // homelab process that set is small and bounded by real sessions; cleanup is -// intentionally omitted (a sweep would race with acquire/release and the +// intentionally omitted (a sweep would race with Acquire/Release and the // memory is negligible). -type turnGate struct { +type TurnGate struct { mu sync.Mutex permits map[string]chan struct{} } -func newTurnGate() *turnGate { - return &turnGate{permits: make(map[string]chan struct{})} +func New() *TurnGate { + return &TurnGate{permits: make(map[string]chan struct{})} } // permit returns the single token-channel for sessionID, creating and seeding // it on first use. Creation is guarded so two concurrent first-callers for the // same id share one channel. -func (g *turnGate) permit(sessionID string) chan struct{} { +func (g *TurnGate) permit(sessionID string) chan struct{} { g.mu.Lock() defer g.mu.Unlock() ch, ok := g.permits[sessionID] @@ -55,11 +55,11 @@ func (g *turnGate) permit(sessionID string) chan struct{} { return ch } -// acquire takes the session's permit. With wait <= 0 it is non-blocking +// Acquire takes the session's permit. With wait <= 0 it is non-blocking // (returns false immediately if a turn is active). With wait > 0 it blocks up // to wait for the permit, returning false on timeout. Every true return MUST -// be paired with exactly one release. -func (g *turnGate) acquire(sessionID string, wait time.Duration) bool { +// be paired with exactly one Release. +func (g *TurnGate) Acquire(sessionID string, wait time.Duration) bool { ch := g.permit(sessionID) if wait <= 0 { select { @@ -79,9 +79,9 @@ func (g *turnGate) acquire(sessionID string, wait time.Duration) bool { } } -// release returns the session's permit. Idempotent: a release with no matching -// acquire (or a double release) is a no-op rather than a blocking send. -func (g *turnGate) release(sessionID string) { +// Release returns the session's permit. Idempotent: a Release with no matching +// Acquire (or a double Release) is a no-op rather than a blocking send. +func (g *TurnGate) Release(sessionID string) { ch := g.permit(sessionID) select { case ch <- struct{}{}: diff --git a/cmd/nomos/turngate_test.go b/internal/nomos/turngate/turngate_test.go similarity index 56% rename from cmd/nomos/turngate_test.go rename to internal/nomos/turngate/turngate_test.go index f230c8cd..c8ecadcc 100644 --- a/cmd/nomos/turngate_test.go +++ b/internal/nomos/turngate/turngate_test.go @@ -1,4 +1,4 @@ -package main +package turngate import ( "sync" @@ -8,74 +8,74 @@ import ( ) func TestTurnGate_NonBlockingSkipsWhenBusy(t *testing.T) { - g := newTurnGate() - if !g.acquire("s1", 0) { - t.Fatal("first non-blocking acquire should succeed on a free session") + g := New() + if !g.Acquire("s1", 0) { + t.Fatal("first non-blocking Acquire should succeed on a free session") } - // A second non-blocking acquire (a background resume) must skip, not queue. - if g.acquire("s1", 0) { - t.Fatal("second non-blocking acquire should fail while a turn is active") + // A second non-blocking Acquire (a background resume) must skip, not queue. + if g.Acquire("s1", 0) { + t.Fatal("second non-blocking Acquire should fail while a turn is active") } // A different session is independent. - if !g.acquire("s2", 0) { - t.Fatal("acquire on a different session should succeed") + if !g.Acquire("s2", 0) { + t.Fatal("Acquire on a different session should succeed") } - g.release("s2") - g.release("s1") - // After release, the session is free again. - if !g.acquire("s1", 0) { - t.Fatal("acquire should succeed again after release") + g.Release("s2") + g.Release("s1") + // After Release, the session is free again. + if !g.Acquire("s1", 0) { + t.Fatal("Acquire should succeed again after Release") } - g.release("s1") + g.Release("s1") } func TestTurnGate_BlockingAcquireWaitsForRelease(t *testing.T) { - g := newTurnGate() - if !g.acquire("s1", 0) { - t.Fatal("first acquire should succeed") + g := New() + if !g.Acquire("s1", 0) { + t.Fatal("first Acquire should succeed") } got := make(chan bool, 1) - go func() { got <- g.acquire("s1", 2*time.Second) }() + go func() { got <- g.Acquire("s1", 2*time.Second) }() select { case <-got: - t.Fatal("blocking acquire should wait, not return before release") + t.Fatal("blocking Acquire should wait, not return before Release") case <-time.After(50 * time.Millisecond): // expected: still waiting } - g.release("s1") + g.Release("s1") select { case ok := <-got: if !ok { - t.Fatal("blocking acquire should succeed after release") + t.Fatal("blocking Acquire should succeed after Release") } case <-time.After(time.Second): - t.Fatal("blocking acquire did not return after release") + t.Fatal("blocking Acquire did not return after Release") } - g.release("s1") + g.Release("s1") } func TestTurnGate_BlockingAcquireTimesOut(t *testing.T) { - g := newTurnGate() - g.acquire("s1", 0) // hold the permit + g := New() + g.Acquire("s1", 0) // hold the permit start := time.Now() - if g.acquire("s1", 60*time.Millisecond) { - t.Fatal("acquire should time out while permit is held") + if g.Acquire("s1", 60*time.Millisecond) { + t.Fatal("Acquire should time out while permit is held") } if elapsed := time.Since(start); elapsed < 50*time.Millisecond { - t.Fatalf("acquire returned too fast (%v); expected to wait ~60ms", elapsed) + t.Fatalf("Acquire returned too fast (%v); expected to wait ~60ms", elapsed) } - g.release("s1") + g.Release("s1") } // TestTurnGate_SingleFlightConcurrent is the core F1 guarantee: many concurrent // background acquirers on the SAME session, exactly one runs at a time. This is // the property that prevents two turns interleaving tool calls. func TestTurnGate_SingleFlightConcurrent(t *testing.T) { - g := newTurnGate() + g := New() const n = 50 var inFlight, maxInFlight int64 var runs int64 @@ -86,10 +86,10 @@ func TestTurnGate_SingleFlightConcurrent(t *testing.T) { go func() { defer wg.Done() <-start - if !g.acquire("shared", 0) { // background-style: skip if busy + if !g.Acquire("shared", 0) { // background-style: skip if busy return } - defer g.release("shared") + defer g.Release("shared") cur := atomic.AddInt64(&inFlight, 1) for { m := atomic.LoadInt64(&maxInFlight)