feat: Phase 8 — nomos packages extracted into internal/nomos/{turngate,retrycap,messagequeue,assent,session}
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.
This commit is contained in:
@@ -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
|
||||
}
|
||||
|
||||
119
internal/migrate/migrate.go
Normal file
119
internal/migrate/migrate.go
Normal file
@@ -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
|
||||
}
|
||||
93
internal/migrate/split.go
Normal file
93
internal/migrate/split.go
Normal file
@@ -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
|
||||
}
|
||||
@@ -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))
|
||||
}
|
||||
143
internal/nomos/assent/assent.go
Normal file
143
internal/nomos/assent/assent.go
Normal file
@@ -0,0 +1,143 @@
|
||||
package assent
|
||||
|
||||
import (
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Chat-assent approval: the operator authorizes a proposed action by
|
||||
// replying normally in chat ("go ahead", "yes", "do it") instead of clicking
|
||||
// a separate Approve button. This is deterministic (not LLM-judged) so it
|
||||
// can't be talked around by a model that misreads intent, and it only ever
|
||||
// looks at the assistant turn immediately preceding the operator's reply —
|
||||
// an old "yes" from three messages ago can never retroactively approve
|
||||
// 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
|
||||
// assistant turn, extracted from its tool_result text.
|
||||
type PendingApproval struct {
|
||||
ExecID string
|
||||
Destructive bool
|
||||
}
|
||||
|
||||
// executionQueuedRE matches the "execution <uuid> queued" phrasing shared by
|
||||
// the run and request_execution/pct_create tool result messages.
|
||||
var executionQueuedRE = regexp.MustCompile(`(?i)execution\s+([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})\s+queued`)
|
||||
|
||||
// extractPendingApprovals scans the tool results of one assistant turn for
|
||||
// gated actions that are still awaiting a decision.
|
||||
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"),
|
||||
})
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// negationWords, checked first: any of these anywhere in the message means
|
||||
// the reply is NOT assent, even if a positive word also appears (e.g. "no,
|
||||
// don't restart it yet" contains neither "yes" nor "go ahead", but "wait"
|
||||
// alone should also block a stray "yes" a sentence later — checking negation
|
||||
// first and returning false errs toward re-confirming rather than assuming
|
||||
// consent, per "when in doubt, escalate"). Includes contracted negatives
|
||||
// ("haven't", "isn't", ...) alongside "don't"/"do not" — found live: "I
|
||||
// haven't confirmed anything yet" was reading as an explicit confirmation
|
||||
// because none of the contracted forms were covered, only "don't"/"do not".
|
||||
// Deliberately does NOT include a bare "not": that's broad enough to false-
|
||||
// negative ordinary assent ("go ahead, this is not risky") — the specific
|
||||
// contracted-verb forms below are unambiguous negation on their own.
|
||||
var negationWords = []string{
|
||||
"no", "nope", "don't", "do not", "stop", "wait", "hold on", "hold off",
|
||||
"not yet", "cancel", "nevermind", "never mind", "actually don't", "skip that",
|
||||
"haven't", "hasn't", "isn't", "wasn't", "aren't", "can't", "cannot",
|
||||
"won't", "wouldn't", "shouldn't", "didn't", "doesn't",
|
||||
}
|
||||
|
||||
// assentWords, checked only if no negation matched.
|
||||
var assentWords = []string{
|
||||
"go ahead", "goahead", "yes", "yep", "yeah", "yup", "do it", "proceed",
|
||||
"approve", "approved", "confirm", "confirmed", "ship it", "sounds good",
|
||||
"lgtm", "run it", "execute", "ok go", "okay go", "please do",
|
||||
}
|
||||
|
||||
// wordTokenRe splits a message into lowercase word tokens. Apostrophes
|
||||
// (straight ' and curly ’) stay attached to their word so "don't"/"haven't"
|
||||
// tokenize as one token, not two.
|
||||
var wordTokenRe = regexp.MustCompile(`[a-z0-9'’]+`)
|
||||
|
||||
func tokenize(msg string) []string {
|
||||
return wordTokenRe.FindAllString(strings.ToLower(strings.ReplaceAll(msg, "’", "'")), -1)
|
||||
}
|
||||
|
||||
// containsPhrase reports whether phrase (one or more words) appears as a
|
||||
// consecutive run of WHOLE tokens in tokens — never a mid-word substring
|
||||
// match. This is the fix for a real false positive found live: the old
|
||||
// substring check (`strings.Contains(m, "yes")`) matched "yes" inside
|
||||
// "yesterday", and "confirm" inside "confirmed"/"unconfirmed" without regard
|
||||
// for word boundaries. Negation already used a word-boundary check
|
||||
// (space-padded); assent/confirm words didn't — this brings both onto the
|
||||
// same, more robust tokenized comparison instead of ad-hoc string padding.
|
||||
func containsPhrase(tokens []string, phrase string) bool {
|
||||
words := strings.Fields(phrase)
|
||||
if len(words) == 0 || len(words) > len(tokens) {
|
||||
return false
|
||||
}
|
||||
for i := 0; i+len(words) <= len(tokens); i++ {
|
||||
match := true
|
||||
for j, w := range words {
|
||||
if tokens[i+j] != w {
|
||||
match = false
|
||||
break
|
||||
}
|
||||
}
|
||||
if match {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isAssent reports whether msg is a plain-language authorization of a
|
||||
// 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 {
|
||||
tokens := tokenize(msg)
|
||||
for _, w := range negationWords {
|
||||
if containsPhrase(tokens, w) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
for _, w := range assentWords {
|
||||
if containsPhrase(tokens, w) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// isTypedConfirmation reports whether msg is an explicit confirmation strong
|
||||
// enough to grant a DESTRUCTIVE pending action. Deliberately a separate,
|
||||
// stricter check from isAssent: a bare "yes"/"go ahead"/"proceed" must never
|
||||
// grant something destructive, only an explicit "confirm" statement does —
|
||||
// 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 {
|
||||
tokens := tokenize(msg)
|
||||
for _, w := range negationWords {
|
||||
if containsPhrase(tokens, w) {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return containsPhrase(tokens, "confirm") || containsPhrase(tokens, "confirmed")
|
||||
}
|
||||
|
||||
128
internal/nomos/assent/assent_test.go
Normal file
128
internal/nomos/assent/assent_test.go
Normal file
@@ -0,0 +1,128 @@
|
||||
package assent
|
||||
|
||||
import (
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestIsAssent_Positive(t *testing.T) {
|
||||
cases := []string{
|
||||
"go ahead", "Go ahead.", "yes", "Yes!", "yeah", "yep", "do it",
|
||||
"proceed", "approve", "ship it", "sounds good", "lgtm", "please do",
|
||||
"ok go ahead and run it",
|
||||
}
|
||||
for _, c := range cases {
|
||||
if !isAssent(c) {
|
||||
t.Errorf("isAssent(%q) = false, want true", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsAssent_Negative(t *testing.T) {
|
||||
cases := []string{
|
||||
"no", "no, don't", "wait", "hold on", "not yet", "cancel that",
|
||||
"nevermind", "what's the plan for tomorrow?", "how many CPUs does strong have?",
|
||||
"maybe later", "",
|
||||
}
|
||||
for _, c := range cases {
|
||||
if isAssent(c) {
|
||||
t.Errorf("isAssent(%q) = true, want false", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsAssent_NegationBeatsAssentWord(t *testing.T) {
|
||||
// Contains "yes" as a substring pattern risk word but is clearly not
|
||||
// assent — negation must win.
|
||||
cases := []string{
|
||||
"no, don't do it yet",
|
||||
"wait, not yet please",
|
||||
}
|
||||
for _, c := range cases {
|
||||
if isAssent(c) {
|
||||
t.Errorf("isAssent(%q) = true, want false (negation should block)", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsAssent_WholeWordBoundary regression-tests a real false positive found
|
||||
// live: the old substring check matched "yes" inside "yesterday" (and would
|
||||
// equally match "confirm" inside "confirmed"/"unconfirmed" for
|
||||
// isTypedConfirmation below) because only negation used a word-boundary
|
||||
// check — assent/confirm words used a bare strings.Contains. Confirmed via a
|
||||
// throwaway probe before being fixed; kept here permanently so a future
|
||||
// change can't silently reintroduce it.
|
||||
func TestIsAssent_WholeWordBoundary(t *testing.T) {
|
||||
cases := []string{
|
||||
"not sure, maybe yesterday's logs show something useful",
|
||||
"my eyesight isn't great, what does that say",
|
||||
}
|
||||
for _, c := range cases {
|
||||
if isAssent(c) {
|
||||
t.Errorf("isAssent(%q) = true, want false (word-boundary: 'yes' must not match inside 'yesterday'/'eyesight')", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsTypedConfirmation_ContractedNegation regression-tests the other real
|
||||
// false positive: isTypedConfirmation gates DESTRUCTIVE actions, and
|
||||
// "confirm" matching inside "confirmed" combined with contracted negatives
|
||||
// ("haven't") not being in negationWords meant a message that explicitly
|
||||
// says the operator has NOT confirmed something could read as confirming it.
|
||||
func TestIsTypedConfirmation_ContractedNegation(t *testing.T) {
|
||||
cases := []string{
|
||||
"I haven't confirmed anything yet, let me think",
|
||||
"that isn't confirmed on my end",
|
||||
"we can't confirm that until tomorrow",
|
||||
}
|
||||
for _, c := range cases {
|
||||
if isTypedConfirmation(c) {
|
||||
t.Errorf("isTypedConfirmation(%q) = true, want false (contracted negation should block)", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsTypedConfirmation(t *testing.T) {
|
||||
positive := []string{
|
||||
"I confirm destroy 135 in strong",
|
||||
"confirm",
|
||||
"Confirmed.",
|
||||
"yes I confirm",
|
||||
}
|
||||
for _, c := range positive {
|
||||
if !isTypedConfirmation(c) {
|
||||
t.Errorf("isTypedConfirmation(%q) = false, want true", c)
|
||||
}
|
||||
}
|
||||
negative := []string{
|
||||
"yes", "go ahead", "do it", "proceed", "lgtm", // loose assent must NOT satisfy this
|
||||
"no, don't confirm yet", "wait", "",
|
||||
}
|
||||
for _, c := range negative {
|
||||
if isTypedConfirmation(c) {
|
||||
t.Errorf("isTypedConfirmation(%q) = true, want false (only explicit confirm should pass)", c)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractPendingApprovals(t *testing.T) {
|
||||
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", len(got))
|
||||
}
|
||||
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 {
|
||||
t.Errorf("second approval should be flagged destructive: %+v", got[1])
|
||||
}
|
||||
}
|
||||
|
||||
func TestExtractPendingApprovals_NoneWhenNoneQueued(t *testing.T) {
|
||||
if got := ExtractPendingApprovals(nil); len(got) != 0 {
|
||||
t.Errorf("expected 0, got %d", len(got))
|
||||
}
|
||||
}
|
||||
82
internal/nomos/messagequeue/messagequeue.go
Normal file
82
internal/nomos/messagequeue/messagequeue.go
Normal file
@@ -0,0 +1,82 @@
|
||||
package messagequeue
|
||||
|
||||
import (
|
||||
"log/slog"
|
||||
"sync"
|
||||
)
|
||||
|
||||
// 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
|
||||
// from the transcript — it just won't auto-run).
|
||||
const MaxQueuedPerSession = 20
|
||||
|
||||
// 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
|
||||
// the session's turn-gate permit.
|
||||
//
|
||||
// The queue only schedules WHEN a turn runs, not WHETHER the message is stored
|
||||
// — handleChat persists the user message before acquiring the gate, so a queued
|
||||
// message is already in the transcript; this just makes sure a turn eventually
|
||||
// acts on it.
|
||||
//
|
||||
// Draining is strictly one-at-a-time under the turn gate (see drainQueued in
|
||||
// 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 {
|
||||
mu sync.Mutex
|
||||
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
|
||||
// message is already persisted in the DB, so this only skips auto-running it.
|
||||
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)
|
||||
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
|
||||
// ok=false when empty.
|
||||
func (q *MessageQueue) Dequeue(sessionID string) (string, bool) {
|
||||
q.mu.Lock()
|
||||
defer q.mu.Unlock()
|
||||
xs := q.queue[sessionID]
|
||||
if len(xs) == 0 {
|
||||
return "", false
|
||||
}
|
||||
m := xs[0]
|
||||
q.queue[sessionID] = xs[1:]
|
||||
return m, true
|
||||
}
|
||||
|
||||
// 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) {
|
||||
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 {
|
||||
q.mu.Lock()
|
||||
defer q.mu.Unlock()
|
||||
return len(q.queue[sessionID])
|
||||
}
|
||||
98
internal/nomos/messagequeue/messagequeue_test.go
Normal file
98
internal/nomos/messagequeue/messagequeue_test.go
Normal file
@@ -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)
|
||||
}
|
||||
}
|
||||
170
internal/nomos/retrycap/retrycap.go
Normal file
170
internal/nomos/retrycap/retrycap.go
Normal file
@@ -0,0 +1,170 @@
|
||||
package retrycap
|
||||
|
||||
import (
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
"strings"
|
||||
"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
|
||||
// loop refuses to dispatch the call again and instead surfaces a directive
|
||||
// to investigate *why* (ps/strace/lsof) or escalate to the operator.
|
||||
//
|
||||
// Background: session 1e9c7691 (2026-07-18) retried the same
|
||||
// `chown :10000 /mnt/media_local && chmod 2775 …` ~20 times across direct
|
||||
// runs, SSH-hop-via-hubris, wrapping in a shell script, and bare `echo test`
|
||||
// sanity checks. Each retry piled up another zombie process on the target
|
||||
// (knfsd was holding a kernel lock on the exported directory). The agent
|
||||
// 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
|
||||
|
||||
// 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
|
||||
// permanently block the agent from ever attempting the operation again.
|
||||
//
|
||||
// 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 {
|
||||
mu sync.Mutex
|
||||
counts 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
|
||||
// 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 {
|
||||
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
|
||||
// dispatching to decide whether to skip the call.
|
||||
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 {
|
||||
r.mu.Lock()
|
||||
defer r.mu.Unlock()
|
||||
return r.counts[key]
|
||||
}
|
||||
|
||||
// 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 <target>: ERROR" — the
|
||||
// shape classifyAndGate/sshExec produce when SSH or the command fails.
|
||||
//
|
||||
// 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`.
|
||||
//
|
||||
// The resultText parameter is the MCP tool's RAW text result (not JSON-
|
||||
// re-encoded): when classifyAndGate returns a textResult like
|
||||
// "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 {
|
||||
if callErr != nil {
|
||||
return true
|
||||
}
|
||||
if toolName != "run" {
|
||||
return false
|
||||
}
|
||||
// "run on host:strong: ERROR ..." or "run on lxc:caddy: ERROR ..."
|
||||
// Both shapes start with "run on ".
|
||||
if !strings.HasPrefix(resultText, "run on ") {
|
||||
return false
|
||||
}
|
||||
return strings.Contains(resultText, ": ERROR")
|
||||
}
|
||||
|
||||
// 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 {
|
||||
switch v := result.(type) {
|
||||
case string:
|
||||
return v
|
||||
case []string:
|
||||
if len(v) > 0 {
|
||||
return v[0]
|
||||
}
|
||||
case []any:
|
||||
var b strings.Builder
|
||||
for _, e := range v {
|
||||
if s, ok := e.(string); ok {
|
||||
b.WriteString(s)
|
||||
}
|
||||
}
|
||||
return b.String()
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
// 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 {
|
||||
return "Refused: this `run` against " + target + " has failed " +
|
||||
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 " +
|
||||
"different routing or quoting. Instead, BEFORE calling `run` again, " +
|
||||
"investigate *why* the command hangs: e.g. `ps aux | grep <cmd>`, " +
|
||||
"`lsof <path>`, `strace -f -p <pid>` or `strace -f <cmd>`, " +
|
||||
"`mount | grep <path>`, `dmesg | tail`. If you find a structural " +
|
||||
"blocker (e.g. a kernel lock on an exported NFS directory → " +
|
||||
"unexport → mutate → re-export), say so to the operator and fix it " +
|
||||
"with a different command. If you genuinely cannot diagnose, " +
|
||||
"surface the blocker to the operator with what you've tried — do " +
|
||||
"not just retry the same command."
|
||||
}
|
||||
|
||||
// itoa is a tiny strconv.Itoa to keep this file dependency-free.
|
||||
func itoa(n int) string {
|
||||
if n == 0 {
|
||||
return "0"
|
||||
}
|
||||
neg := n < 0
|
||||
if neg {
|
||||
n = -n
|
||||
}
|
||||
var buf [20]byte
|
||||
i := len(buf)
|
||||
for n > 0 {
|
||||
i--
|
||||
buf[i] = byte('0' + n%10)
|
||||
n /= 10
|
||||
}
|
||||
if neg {
|
||||
i--
|
||||
buf[i] = '-'
|
||||
}
|
||||
return string(buf[i:])
|
||||
}
|
||||
129
internal/nomos/retrycap/retrycap_test.go
Normal file
129
internal/nomos/retrycap/retrycap_test.go
Normal file
@@ -0,0 +1,129 @@
|
||||
package retrycap
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
func TestRunFailureKey_StableAcrossWhitespace(t *testing.T) {
|
||||
cases := []struct{ a, b string }{
|
||||
{"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local",
|
||||
"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local"},
|
||||
{"chown :10000 /mnt/media_local\n&& chmod 2775 /mnt/media_local",
|
||||
"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local"},
|
||||
{"chown :10000 /mnt/media_local && chmod 2775 /mnt/media_local ",
|
||||
" 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)
|
||||
if ka != kb {
|
||||
t.Errorf("case %d: keys differ for whitespace-equivalent commands:\n a=%q\n b=%q", i, c.a, c.b)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunFailureKey_DiffersByTarget(t *testing.T) {
|
||||
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")
|
||||
if a == b {
|
||||
t.Error("keys should differ when command differs")
|
||||
}
|
||||
}
|
||||
|
||||
func TestRunRetryTracker_CountsAndCaps(t *testing.T) {
|
||||
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)
|
||||
}
|
||||
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 := 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)
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsRunFailure(t *testing.T) {
|
||||
cases := []struct {
|
||||
desc string
|
||||
tool string
|
||||
result string
|
||||
callErr error
|
||||
want bool
|
||||
}{
|
||||
{"run with ERROR prefix", "run", "run on host:strong: ERROR ssh: signal: killed", nil, true},
|
||||
{"run with exit error", "run", "run on lxc:caddy: ERROR exit status 1", nil, true},
|
||||
{"run success (read-only auto)", "run", "run on host:strong (read_only, auto): hello", nil, false},
|
||||
{"run success (assent window)", "run", "run on host:strong (config_mutation, auto via assent window): done", nil, false},
|
||||
{"run queued for approval", "run", "run on host:strong requires approval (risk: config_mutation) — execution 019f4930 queued. Present the command and purpose to the operator and wait; do not re-request.", nil, false},
|
||||
{"non-run tool", "get_entity", "lxc list result", nil, false},
|
||||
{"callErr set (dispatch failure)", "run", "", errFake{}, true},
|
||||
{"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)
|
||||
if got != c.want {
|
||||
t.Errorf("case %d (%s): IsRunFailure = %v, want %v", i, c.desc, got, c.want)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
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)
|
||||
for _, want := range []string{
|
||||
"Refused:",
|
||||
"host:strong",
|
||||
"3 times",
|
||||
"retry cap hit",
|
||||
"Do NOT retry",
|
||||
"strace",
|
||||
"ps aux",
|
||||
"lsof",
|
||||
"surface the blocker",
|
||||
} {
|
||||
if !strings.Contains(d, want) {
|
||||
t.Errorf("directive missing %q; got:\n%s", want, d)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestItoa(t *testing.T) {
|
||||
cases := map[int]string{0: "0", 1: "1", 9: "9", 10: "10", 42: "42",
|
||||
100: "100", -1: "-1", -42: "-42"}
|
||||
for in, want := range cases {
|
||||
if got := itoa(in); got != want {
|
||||
t.Errorf("itoa(%d) = %q, want %q", in, got, want)
|
||||
}
|
||||
}
|
||||
}
|
||||
2145
internal/nomos/session/store.go
Normal file
2145
internal/nomos/session/store.go
Normal file
File diff suppressed because it is too large
Load Diff
511
internal/nomos/session/store_test.go
Normal file
511
internal/nomos/session/store_test.go
Normal file
@@ -0,0 +1,511 @@
|
||||
package main
|
||||
|
||||
// Integration tests against a real Postgres, mirroring
|
||||
// internal/db/integration_test.go's pattern: guarded by
|
||||
// OIKOS_TEST_DATABASE_URL (skipped when unset), throwaway database per run,
|
||||
// full migrations applied, dropped on cleanup. Run with:
|
||||
//
|
||||
// docker compose up -d postgres
|
||||
// OIKOS_TEST_DATABASE_URL="postgres://oikos:oikos_dev@localhost:5432/oikos?sslmode=disable" go test ./cmd/nomos/
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"math/rand"
|
||||
"os"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/dtoro/oikos/internal/adapters/postgres"
|
||||
"github.com/google/uuid"
|
||||
"github.com/jackc/pgx/v5"
|
||||
)
|
||||
|
||||
// newTestStore creates a throwaway, fully-migrated database and returns a
|
||||
// *Store connected to it, cleaned up (including a matching task:<session>
|
||||
// entity type in the ontology, needed by createTaskEntity/proposePlan tests)
|
||||
// via t.Cleanup.
|
||||
func newTestStore(t *testing.T) *Store {
|
||||
t.Helper()
|
||||
baseURL := os.Getenv("OIKOS_TEST_DATABASE_URL")
|
||||
if baseURL == "" {
|
||||
t.Skip("OIKOS_TEST_DATABASE_URL not set — skipping integration test")
|
||||
}
|
||||
ctx := context.Background()
|
||||
|
||||
admin, err := pgx.Connect(ctx, baseURL)
|
||||
if err != nil {
|
||||
t.Fatalf("connect admin: %v", err)
|
||||
}
|
||||
dbName := fmt.Sprintf("oikos_test_nomos_%08x", rand.Int63())
|
||||
if _, err := admin.Exec(ctx, "CREATE DATABASE "+dbName); err != nil {
|
||||
admin.Close(ctx)
|
||||
t.Fatalf("create test db: %v", err)
|
||||
}
|
||||
admin.Close(ctx)
|
||||
|
||||
testURL := swapTestDatabase(baseURL, dbName)
|
||||
pool, err := db.New(ctx, testURL)
|
||||
if err != nil {
|
||||
t.Fatalf("connect test db: %v", err)
|
||||
}
|
||||
t.Cleanup(func() {
|
||||
pool.Close()
|
||||
admin, err := pgx.Connect(ctx, baseURL)
|
||||
if err == nil {
|
||||
admin.Exec(ctx, "DROP DATABASE IF EXISTS "+dbName+" WITH (FORCE)")
|
||||
admin.Close(ctx)
|
||||
}
|
||||
})
|
||||
if err := pool.Migrate(ctx); err != nil {
|
||||
t.Fatalf("migrate: %v", err)
|
||||
}
|
||||
|
||||
// session_plan_steps/session_questions tests don't need the ontology
|
||||
// seed, but createTaskEntity's INSERT INTO entities (type='task') has an
|
||||
// FK to entity_types — seed the minimal rows it needs directly rather
|
||||
// than pulling in the full seeds/ontology.yaml ingest path.
|
||||
if _, err := pool.Exec(ctx, `
|
||||
INSERT INTO entity_types (name, domain, layer) VALUES ('entity', 'meta', 'meta')
|
||||
ON CONFLICT DO NOTHING;
|
||||
INSERT INTO entity_types (name, parent_type, domain, layer) VALUES ('task', 'entity', 'cognition', 'cognition')
|
||||
ON CONFLICT DO NOTHING;`); err != nil {
|
||||
t.Fatalf("seed minimal ontology: %v", err)
|
||||
}
|
||||
|
||||
return &Store{pool: pool.Pool}
|
||||
}
|
||||
|
||||
func swapTestDatabase(url, dbName string) string {
|
||||
qi := strings.Index(url, "?")
|
||||
params, base := "", url
|
||||
if qi >= 0 {
|
||||
params = url[qi:]
|
||||
base = url[:qi]
|
||||
}
|
||||
si := strings.LastIndex(base, "/")
|
||||
return base[:si+1] + dbName + params
|
||||
}
|
||||
|
||||
// TestGetRecentMessages_Truncation is the concrete proof for fix A2 of
|
||||
// plans/2026-07-11-nomos-agent-code-review.md: chatWith used to replay a
|
||||
// session's ENTIRE history on every turn with no bound. getRecentMessages
|
||||
// caps that; this test checks both sides — under the limit, nothing is
|
||||
// dropped and truncated=false; over it, only the most recent `limit` come
|
||||
// back, in chronological order, with truncated=true.
|
||||
func TestGetRecentMessages_Truncation(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sess, err := s.createSession(ctx, "history window test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
|
||||
const total = 35
|
||||
const limit = 30
|
||||
for i := 0; i < total; i++ {
|
||||
role := "user"
|
||||
if i%2 == 1 {
|
||||
role = "assistant"
|
||||
}
|
||||
body := fmt.Appendf(nil, `{"role":%q,"text":"msg-%d"}`, role, i)
|
||||
if err := s.saveMessage(ctx, sess.ID, role, body); err != nil {
|
||||
t.Fatalf("saveMessage %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
|
||||
msgs, truncated, err := s.getRecentMessages(ctx, sess.ID, limit)
|
||||
if err != nil {
|
||||
t.Fatalf("getRecentMessages: %v", err)
|
||||
}
|
||||
if !truncated {
|
||||
t.Errorf("truncated = false, want true (%d messages > limit %d)", total, limit)
|
||||
}
|
||||
if len(msgs) != limit {
|
||||
t.Fatalf("got %d messages, want %d", len(msgs), limit)
|
||||
}
|
||||
// Chronological order: the oldest of the RETAINED messages should be the
|
||||
// (total-limit)-th one saved (msg-5, since msg-0..4 were dropped), and
|
||||
// the last should be the most recently saved (msg-34).
|
||||
wantFirst := fmt.Sprintf("msg-%d", total-limit)
|
||||
wantLast := fmt.Sprintf("msg-%d", total-1)
|
||||
if got := extractText(msgs[0].Content); got != wantFirst {
|
||||
t.Errorf("first retained message = %q, want %q", got, wantFirst)
|
||||
}
|
||||
if got := extractText(msgs[len(msgs)-1].Content); got != wantLast {
|
||||
t.Errorf("last retained message = %q, want %q", got, wantLast)
|
||||
}
|
||||
|
||||
// Under the limit: nothing dropped.
|
||||
sess2, err := s.createSession(ctx, "small session")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
for i := 0; i < 5; i++ {
|
||||
body := fmt.Appendf(nil, `{"role":"user","text":"msg-%d"}`, i)
|
||||
if err := s.saveMessage(ctx, sess2.ID, "user", body); err != nil {
|
||||
t.Fatalf("saveMessage: %v", err)
|
||||
}
|
||||
}
|
||||
msgs2, truncated2, err := s.getRecentMessages(ctx, sess2.ID, limit)
|
||||
if err != nil {
|
||||
t.Fatalf("getRecentMessages (small): %v", err)
|
||||
}
|
||||
if truncated2 {
|
||||
t.Errorf("truncated = true for a 5-message session under a %d limit, want false", limit)
|
||||
}
|
||||
if len(msgs2) != 5 {
|
||||
t.Errorf("got %d messages, want 5", len(msgs2))
|
||||
}
|
||||
}
|
||||
|
||||
// TestProposePlan_RefuseInFlight is the concrete proof for the plan-drift
|
||||
// fix (2026-07-14, "plan added twice in the sidebar"): proposePlan must
|
||||
// REPLACE the step list only while every existing step is still 'pending'
|
||||
// (a genuine pre-execution revision), and REFUSE the call once any step has
|
||||
// started. The prior append-mode safety net (commit 5384499) preserved
|
||||
// history but duplicated the plan in the sidebar when the agent re-proposed
|
||||
// on "proceed". Refusing is the correct default — the agent must advance
|
||||
// with update_plan_step + run.
|
||||
func TestProposePlan_RefuseInFlight(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sess, err := s.createSession(ctx, "plan refuse test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
|
||||
// First call: no steps exist yet — must persist as-is (replace mode,
|
||||
// trivially: nothing to replace).
|
||||
out1, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "Step A"}})
|
||||
if err != nil {
|
||||
t.Fatalf("proposePlan #1: %v", err)
|
||||
}
|
||||
if len(out1) != 1 || out1[0]["seq"] != 1 {
|
||||
t.Fatalf("proposePlan #1 = %+v, want one step at seq 1", out1)
|
||||
}
|
||||
if out1[0]["generation"] != 1 {
|
||||
t.Fatalf("proposePlan #1 generation = %v, want 1", out1[0]["generation"])
|
||||
}
|
||||
|
||||
// Mark step 1 as started.
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep: %v", err)
|
||||
}
|
||||
|
||||
// Second call, simulating a model that re-proposes mid-flight (the
|
||||
// operator-reported "proceed" bug): since step 1 has left 'pending',
|
||||
// this MUST refuse with errPlanInFlight, not append or replace.
|
||||
_, err = s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "Step B"}})
|
||||
if !errors.Is(err, errPlanInFlight) {
|
||||
t.Fatalf("proposePlan #2: err = %v, want errPlanInFlight (refuse mid-flight re-proposal)", err)
|
||||
}
|
||||
|
||||
// The original step 1 must be untouched — not erased, not appended to.
|
||||
steps, err := s.getPlanSteps(ctx, sess.ID, false)
|
||||
if err != nil {
|
||||
t.Fatalf("getPlanSteps: %v", err)
|
||||
}
|
||||
if len(steps) != 1 {
|
||||
t.Fatalf("got %d persisted steps, want 1 (refused call must not mutate the plan)", len(steps))
|
||||
}
|
||||
if steps[0].Title != "Step A" || steps[0].Status != "running" {
|
||||
t.Errorf("step 1 = %+v, want Step A still running (refused call must not touch it)", steps[0])
|
||||
}
|
||||
|
||||
// Third call BEFORE anything runs on a fresh session: every step is
|
||||
// still pending, so this must REPLACE (mark the prior plan `replaced`),
|
||||
// not refuse. The new plan becomes generation 2.
|
||||
sess2, err := s.createSession(ctx, "plan replace test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
if _, err := s.proposePlan(ctx, sess2.ID, []PlanStepInput{{Title: "Original"}}); err != nil {
|
||||
t.Fatalf("proposePlan (initial): %v", err)
|
||||
}
|
||||
if _, err := s.proposePlan(ctx, sess2.ID, []PlanStepInput{{Title: "Revised"}}); err != nil {
|
||||
t.Fatalf("proposePlan (revise before execution): %v", err)
|
||||
}
|
||||
// Default (current generation) view: only the revised step.
|
||||
revisedSteps, err := s.getPlanSteps(ctx, sess2.ID, false)
|
||||
if err != nil {
|
||||
t.Fatalf("getPlanSteps: %v", err)
|
||||
}
|
||||
if len(revisedSteps) != 1 || revisedSteps[0].Title != "Revised" {
|
||||
t.Fatalf("got %+v, want a single 'Revised' step (current-generation view)", revisedSteps)
|
||||
}
|
||||
if revisedSteps[0].Seq != 1 {
|
||||
t.Fatalf("revised step seq = %d, want 1 (seq is generation-relative, resets to 1..N)", revisedSteps[0].Seq)
|
||||
}
|
||||
if revisedSteps[0].Generation != 2 {
|
||||
t.Fatalf("revised step generation = %d, want 2 (prior pending plan is replaced, not deleted, so the counter increments)", revisedSteps[0].Generation)
|
||||
}
|
||||
// all=true audit view: both generations, the original marked `replaced`.
|
||||
allSteps, err := s.getPlanSteps(ctx, sess2.ID, true)
|
||||
if err != nil {
|
||||
t.Fatalf("getPlanSteps(all): %v", err)
|
||||
}
|
||||
if len(allSteps) != 2 {
|
||||
t.Fatalf("all=true got %d steps, want 2 (Original replaced gen1 + Revised gen2)", len(allSteps))
|
||||
}
|
||||
if allSteps[0].Title != "Original" || allSteps[0].Status != "replaced" || allSteps[0].Generation != 1 {
|
||||
t.Errorf("gen1 step = %+v, want Original/replaced/gen1", allSteps[0])
|
||||
}
|
||||
if allSteps[1].Title != "Revised" || allSteps[1].Generation != 2 || allSteps[1].Seq != 1 {
|
||||
t.Errorf("gen2 step = %+v, want Revised/gen2/seq1", allSteps[1])
|
||||
}
|
||||
}
|
||||
|
||||
// TestUpdatePlanStep_GenerationRelative is the P0.1 regression proof: after a
|
||||
// re-plan, update_plan_step(seq=N) — using the 1-based number the model
|
||||
// naturally carries — must address the CURRENT generation and never resurrect
|
||||
// a superseded generation's `replaced` row. Before the fix, seq was globally
|
||||
// increasing across generations, so seq=1 after a re-plan flipped the gen-1
|
||||
// `replaced` step back to `running`/`done` while the real gen-2 work went
|
||||
// unrecorded.
|
||||
func TestUpdatePlanStep_GenerationRelative(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sess, err := s.createSession(ctx, "gen-relative seq test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
// Generation 1: two steps.
|
||||
if _, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "A"}, {Title: "B"}}); err != nil {
|
||||
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 {
|
||||
t.Fatalf("proposePlan #2: %v", err)
|
||||
}
|
||||
|
||||
// The model addresses the new plan with 1-based seq. seq=1 must hit
|
||||
// gen-2 "C", leaving gen-1 "A" (replaced) untouched.
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep(seq=1, running): %v", err)
|
||||
}
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "done", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep(seq=1, done): %v", err)
|
||||
}
|
||||
|
||||
all, err := s.getPlanSteps(ctx, sess.ID, true)
|
||||
if err != nil {
|
||||
t.Fatalf("getPlanSteps(all): %v", err)
|
||||
}
|
||||
byTitle := map[string]planStep{}
|
||||
for _, st := range all {
|
||||
byTitle[st.Title] = st
|
||||
}
|
||||
// gen-1 steps stay `replaced` — NOT resurrected to running/done.
|
||||
if byTitle["A"].Status != "replaced" || byTitle["A"].Generation != 1 {
|
||||
t.Errorf("A = %+v, want replaced/gen1 (a superseded row must never be touched)", byTitle["A"])
|
||||
}
|
||||
if byTitle["B"].Status != "replaced" || byTitle["B"].Generation != 1 {
|
||||
t.Errorf("B = %+v, want replaced/gen1", byTitle["B"])
|
||||
}
|
||||
// gen-2 seq=1 advanced; seq=2 untouched.
|
||||
if byTitle["C"].Status != "done" || byTitle["C"].Generation != 2 || byTitle["C"].Seq != 1 {
|
||||
t.Errorf("C = %+v, want done/gen2/seq1 (the 1-based update must address the current generation)", byTitle["C"])
|
||||
}
|
||||
if byTitle["D"].Status != "pending" || byTitle["D"].Seq != 2 {
|
||||
t.Errorf("D = %+v, want pending/seq2", byTitle["D"])
|
||||
}
|
||||
|
||||
// Out-of-range seq must be refused (no current-gen step there).
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 99, "running", "", ""); !errors.Is(err, errPlanStepNotFound) {
|
||||
t.Fatalf("updatePlanStep(seq=99) err = %v, want errPlanStepNotFound", err)
|
||||
}
|
||||
}
|
||||
|
||||
// TestCompleteTask_AutoCloseEmitsEvents is the P1.1 regression proof:
|
||||
// completeTask's bulk auto-close of in-flight steps must emit one
|
||||
// plan.step.finished event per closed step (so the live panel converges
|
||||
// instead of freezing on "running" after the task completes) and must stamp
|
||||
// started_at so no closed step is left un-timestamped (P0.1 fix 5).
|
||||
func TestCompleteTask_AutoCloseEmitsEvents(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sess, err := s.createSession(ctx, "auto-close events test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
if _, err := s.proposePlan(ctx, sess.ID, []PlanStepInput{{Title: "A"}, {Title: "B"}}); err != nil {
|
||||
t.Fatalf("proposePlan: %v", err)
|
||||
}
|
||||
// A is running, B still pending at completion time.
|
||||
if err := s.updatePlanStep(ctx, sess.ID, 1, "running", "", ""); err != nil {
|
||||
t.Fatalf("updatePlanStep(1, running): %v", err)
|
||||
}
|
||||
if err := s.completeTask(ctx, sess.ID, "success", "done"); err != nil {
|
||||
t.Fatalf("completeTask: %v", err)
|
||||
}
|
||||
|
||||
// Every auto-closed step should now carry both a started_at and a
|
||||
// finished_at (no NULL-started `done` step).
|
||||
steps, err := s.getPlanSteps(ctx, sess.ID, true)
|
||||
if err != nil {
|
||||
t.Fatalf("getPlanSteps: %v", err)
|
||||
}
|
||||
for _, st := range steps {
|
||||
if st.Status == "done" && st.StartedAt == nil {
|
||||
t.Errorf("step %q done but started_at is NULL (P0.1 fix 5: stamp it)", st.Title)
|
||||
}
|
||||
}
|
||||
|
||||
// Exactly two plan.step.finished events — one per closed step (A and B).
|
||||
var finished int
|
||||
if err := s.pool.QueryRow(ctx,
|
||||
`SELECT COUNT(*) FROM events WHERE type = 'plan.step.finished' AND correlation_id = $1`,
|
||||
sess.ID).Scan(&finished); err != nil {
|
||||
t.Fatalf("count events: %v", err)
|
||||
}
|
||||
if finished != 2 {
|
||||
t.Fatalf("plan.step.finished events = %d, want 2 (one per auto-closed step)", finished)
|
||||
}
|
||||
}
|
||||
|
||||
// TestHadDiscoveryAndWriteback is the Store-level proof for D.1 (refuse
|
||||
// complete_task when discovery ran without writeback). hadDiscovery must
|
||||
// report true only after a successful `run` call; hadEntityWriteback must
|
||||
// report true only after a successful update_entity_attributes or
|
||||
// create_relationship call. The D.1 gate in tasks.go combines these: refuse
|
||||
// success when hadDiscovery && !hadEntityWriteback.
|
||||
func TestHadDiscoveryAndWriteback(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sess, err := s.createSession(ctx, "discovery test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
|
||||
// Before any tool calls: no discovery, no writeback.
|
||||
if s.hadDiscovery(ctx, sess.ID) {
|
||||
t.Fatal("hadDiscovery = true before any tool calls, want false")
|
||||
}
|
||||
if s.hadEntityWriteback(ctx, sess.ID) {
|
||||
t.Fatal("hadEntityWriteback = true before any tool calls, want false")
|
||||
}
|
||||
|
||||
// A `run` call (discovery) — should set hadDiscovery, not hadEntityWriteback.
|
||||
agentID := uuid.New()
|
||||
s.logActivity(ctx, agentID, sess.ID, "run", nil, "", "uptime output", 100, true, "corr-1", 0)
|
||||
if !s.hadDiscovery(ctx, sess.ID) {
|
||||
t.Fatal("hadDiscovery = false after a successful run call, want true")
|
||||
}
|
||||
if s.hadEntityWriteback(ctx, sess.ID) {
|
||||
t.Fatal("hadEntityWriteback = true after only a run call, want false")
|
||||
}
|
||||
|
||||
// A failed run call should NOT count as discovery (no facts learned).
|
||||
sess2, err := s.createSession(ctx, "failed discovery test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
s.logActivity(ctx, agentID, sess2.ID, "run", nil, "", "ssh timeout", 100, false, "corr-2", 0)
|
||||
if s.hadDiscovery(ctx, sess2.ID) {
|
||||
t.Fatal("hadDiscovery = true after a failed run call, want false (no facts learned)")
|
||||
}
|
||||
|
||||
// A get_entity call should NOT count as discovery (DB lookup, not live state).
|
||||
sess3, err := s.createSession(ctx, "lookup test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
s.logActivity(ctx, agentID, sess3.ID, "get_entity", nil, "", "entity row", 10, true, "corr-3", 0)
|
||||
if s.hadDiscovery(ctx, sess3.ID) {
|
||||
t.Fatal("hadDiscovery = true after get_entity, want false (DB lookups are not discovery)")
|
||||
}
|
||||
|
||||
// update_entity_attributes sets hadEntityWriteback.
|
||||
sess4, err := s.createSession(ctx, "writeback test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
s.logActivity(ctx, agentID, sess4.ID, "update_entity_attributes", nil, "", "ok", 10, true, "corr-4", 0)
|
||||
if !s.hadEntityWriteback(ctx, sess4.ID) {
|
||||
t.Fatal("hadEntityWriteback = false after update_entity_attributes, want true")
|
||||
}
|
||||
// And the discovery+writeback combination (the conv3 scenario).
|
||||
s.logActivity(ctx, agentID, sess4.ID, "run", nil, "", "apt-get update output", 100, true, "corr-5", 0)
|
||||
if !s.hadDiscovery(ctx, sess4.ID) {
|
||||
t.Fatal("hadDiscovery = false after run+writeback, want true")
|
||||
}
|
||||
if !s.hadEntityWriteback(ctx, sess4.ID) {
|
||||
t.Fatal("hadEntityWriteback = false after run+writeback, want true")
|
||||
}
|
||||
}
|
||||
|
||||
// TestSetGoal_SupersessionEvent is the Store-level proof for P1.4 from
|
||||
// plans/2026-07-18-session-review-three-sessions.md: when setGoal is called
|
||||
// and a non-empty prior goal already exists with a DIFFERENT value, a
|
||||
// task.superseded event must be emitted (so the audit trail records the
|
||||
// pivot — the row's goal column will be overwritten, losing the prior intent
|
||||
// without this event). When the goal is identical OR no prior goal exists,
|
||||
// no supersession event is emitted.
|
||||
//
|
||||
// Background: session 55927f0a had two set_goal calls; the first was
|
||||
// implicitly abandoned when the operator said "lets just keep ludo-library
|
||||
// then." Without the event, the prior goal silently disappeared.
|
||||
func TestSetGoal_SupersededEvent(t *testing.T) {
|
||||
s := newTestStore(t)
|
||||
ctx := context.Background()
|
||||
|
||||
sess, err := s.createSession(ctx, "goal pivot test")
|
||||
if err != nil {
|
||||
t.Fatalf("createSession: %v", err)
|
||||
}
|
||||
|
||||
// First set_goal — no prior, no supersession event expected.
|
||||
if err := s.setGoal(ctx, sess.ID, "Fix sabnzbd download folder to use ludo-lvm"); err != nil {
|
||||
t.Fatalf("setGoal #1: %v", err)
|
||||
}
|
||||
if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 0 {
|
||||
t.Errorf("after first set_goal: %d task.superseded events, want 0", n)
|
||||
}
|
||||
|
||||
// Second set_goal with a DIFFERENT goal — supersession event expected.
|
||||
if err := s.setGoal(ctx, sess.ID, "Add NFS export of ludo-lvm to ZimaOS"); err != nil {
|
||||
t.Fatalf("setGoal #2: %v", err)
|
||||
}
|
||||
if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 1 {
|
||||
t.Errorf("after second set_goal with a different goal: %d task.superseded events, want 1", n)
|
||||
}
|
||||
|
||||
// Third set_goal with the SAME goal as the second — no new supersession
|
||||
// event (idempotent: same goal is a no-op, not a pivot).
|
||||
if err := s.setGoal(ctx, sess.ID, "Add NFS export of ludo-lvm to ZimaOS"); err != nil {
|
||||
t.Fatalf("setGoal #3: %v", err)
|
||||
}
|
||||
if n := countEvents(ctx, s, sess.ID, "task.superseded"); n != 1 {
|
||||
t.Errorf("after third set_goal with same goal as second: %d task.superseded events, want 1 (no new pivot)", n)
|
||||
}
|
||||
|
||||
// The session's current goal must be the latest one set.
|
||||
got, err := s.getSession(ctx, sess.ID)
|
||||
if err != nil {
|
||||
t.Fatalf("getSession: %v", err)
|
||||
}
|
||||
if got.Goal != "Add NFS export of ludo-lvm to ZimaOS" {
|
||||
t.Errorf("session goal = %q, want the second (latest) goal", got.Goal)
|
||||
}
|
||||
}
|
||||
|
||||
// countEvents counts observability events of the given type correlated to
|
||||
// the given session. Used by TestSetGoal_SupersededEvent to assert the
|
||||
// task.superseded audit-trail signal was emitted.
|
||||
func countEvents(ctx context.Context, s *Store, sessionID, eventType string) int {
|
||||
var n int
|
||||
s.pool.QueryRow(ctx,
|
||||
`SELECT COUNT(*) FROM events WHERE correlation_id = $1 AND type = $2`,
|
||||
sessionID, eventType).Scan(&n)
|
||||
return n
|
||||
}
|
||||
90
internal/nomos/turngate/turngate.go
Normal file
90
internal/nomos/turngate/turngate.go
Normal file
@@ -0,0 +1,90 @@
|
||||
package turngate
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"time"
|
||||
)
|
||||
|
||||
// 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,
|
||||
// the auto-continuation worker, the idle sweep, answer-question, the /resume
|
||||
// endpoint) launches ANOTHER goroutine running a full turn. Nothing prevented
|
||||
// two turns for the SAME session at once, so a network blip that triggered a
|
||||
// reconnect would spawn a duplicate resumeSession while the original turn was
|
||||
// still alive — their tool calls interleaved on the wire and in the persisted
|
||||
// transcript, which is the root cause behind the "parallel/nesting/sequence
|
||||
// is off" and "task didn't end / flaky" reports.
|
||||
//
|
||||
// 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
|
||||
// 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
|
||||
// background turn, then bails with an actionable error if still busy — see
|
||||
// handleChat.
|
||||
//
|
||||
// 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
|
||||
// memory is negligible).
|
||||
type TurnGate struct {
|
||||
mu sync.Mutex
|
||||
permits 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{} {
|
||||
g.mu.Lock()
|
||||
defer g.mu.Unlock()
|
||||
ch, ok := g.permits[sessionID]
|
||||
if !ok {
|
||||
ch = make(chan struct{}, 1)
|
||||
ch <- struct{}{}
|
||||
g.permits[sessionID] = ch
|
||||
}
|
||||
return ch
|
||||
}
|
||||
|
||||
// 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 {
|
||||
ch := g.permit(sessionID)
|
||||
if wait <= 0 {
|
||||
select {
|
||||
case <-ch:
|
||||
return true
|
||||
default:
|
||||
return false
|
||||
}
|
||||
}
|
||||
t := time.NewTimer(wait)
|
||||
defer t.Stop()
|
||||
select {
|
||||
case <-ch:
|
||||
return true
|
||||
case <-t.C:
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
// 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{}{}:
|
||||
default:
|
||||
}
|
||||
}
|
||||
114
internal/nomos/turngate/turngate_test.go
Normal file
114
internal/nomos/turngate/turngate_test.go
Normal file
@@ -0,0 +1,114 @@
|
||||
package turngate
|
||||
|
||||
import (
|
||||
"sync"
|
||||
"sync/atomic"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func TestTurnGate_NonBlockingSkipsWhenBusy(t *testing.T) {
|
||||
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 different session is independent.
|
||||
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("s1")
|
||||
}
|
||||
|
||||
func TestTurnGate_BlockingAcquireWaitsForRelease(t *testing.T) {
|
||||
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) }()
|
||||
|
||||
select {
|
||||
case <-got:
|
||||
t.Fatal("blocking Acquire should wait, not return before Release")
|
||||
case <-time.After(50 * time.Millisecond):
|
||||
// expected: still waiting
|
||||
}
|
||||
|
||||
g.Release("s1")
|
||||
select {
|
||||
case ok := <-got:
|
||||
if !ok {
|
||||
t.Fatal("blocking Acquire should succeed after Release")
|
||||
}
|
||||
case <-time.After(time.Second):
|
||||
t.Fatal("blocking Acquire did not return after Release")
|
||||
}
|
||||
g.Release("s1")
|
||||
}
|
||||
|
||||
func TestTurnGate_BlockingAcquireTimesOut(t *testing.T) {
|
||||
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 elapsed := time.Since(start); elapsed < 50*time.Millisecond {
|
||||
t.Fatalf("Acquire returned too fast (%v); expected to wait ~60ms", elapsed)
|
||||
}
|
||||
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 := New()
|
||||
const n = 50
|
||||
var inFlight, maxInFlight int64
|
||||
var runs int64
|
||||
var wg sync.WaitGroup
|
||||
wg.Add(n)
|
||||
start := make(chan struct{})
|
||||
for i := 0; i < n; i++ {
|
||||
go func() {
|
||||
defer wg.Done()
|
||||
<-start
|
||||
if !g.Acquire("shared", 0) { // background-style: skip if busy
|
||||
return
|
||||
}
|
||||
defer g.Release("shared")
|
||||
cur := atomic.AddInt64(&inFlight, 1)
|
||||
for {
|
||||
m := atomic.LoadInt64(&maxInFlight)
|
||||
if cur <= m || atomic.CompareAndSwapInt64(&maxInFlight, m, cur) {
|
||||
break
|
||||
}
|
||||
}
|
||||
atomic.AddInt64(&runs, 1)
|
||||
time.Sleep(2 * time.Millisecond)
|
||||
atomic.AddInt64(&inFlight, -1)
|
||||
}()
|
||||
}
|
||||
close(start)
|
||||
wg.Wait()
|
||||
|
||||
if maxInFlight != 1 {
|
||||
t.Fatalf("max in-flight turns = %d, want 1 (turns must not overlap)", maxInFlight)
|
||||
}
|
||||
if runs == 0 {
|
||||
t.Fatal("expected at least one turn to run")
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user