diff --git a/CLAUDE.md b/CLAUDE.md index 2ed99a0..2e7f530 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -61,7 +61,9 @@ CORE (majordomo + stdlib): tools/{web,net,store,compose,meta,comms} generic tools [P3] BATTERIES (opt-in siblings, each nil-safe + a default): - persona/ Agent noun + AgentStore seam + yml loader [P4] + persona/ Agent noun + Storage seam + builtin loader [P4 ~] + + ToRunnable() bridge to run.RunnableAgent + + Memory default (host: chatbot/commands/personalization) skill/ rich Skill + SkillStore seam + toml loader [P4] audit/ run.Audit Sink + Writer + queryable Memory [P4 ✓] default (skillaudit Storage iface; GORM stays in mort) diff --git a/go.mod b/go.mod index c8ea9c0..7463def 100644 --- a/go.mod +++ b/go.mod @@ -6,6 +6,7 @@ require ( gitea.stevedudenhoeffer.com/steve/majordomo v0.0.0-20260626223738-1fd7109a42f3 github.com/google/uuid v1.6.0 golang.org/x/crypto v0.53.0 + gopkg.in/yaml.v3 v3.0.1 ) require ( diff --git a/go.sum b/go.sum index 22e9ecc..a7e5c65 100644 --- a/go.sum +++ b/go.sum @@ -125,6 +125,7 @@ google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6h google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= +gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4= diff --git a/persona/agent.go b/persona/agent.go new file mode 100644 index 0000000..19fbce0 --- /dev/null +++ b/persona/agent.go @@ -0,0 +1,191 @@ +// Package agents implements the Agent noun: a persisted persona + +// execution spec + palette of skills/sub-agents/low-level tools, with +// optional trigger metadata (schedule, webhook, chatbot channel +// listener) and personalization sources. +// +// Phase 1 scope: this package introduces Agent as a persisted noun +// with CRUD only — no execution path, no palette resolution, no +// trigger handling. See /Users/steve/.claude/plans/serene-churning-micali.md +// for the staged rollout. Later phases add agentexec, agent_invoke, +// trigger dispatch (schedule/webhook/chatbot), and CommandBinding. +// +// The three-layer storage pattern from pkg/logic/storage/CLAUDE.md +// applies — when adding a field to Agent, you MUST update +// pkg/logic/storage/agents.go (gormAgent, agentFromStorage, +// toStorage) or persistence will silently break. +package persona + +import "time" + +// Agent is the domain definition of an Agent persona + execution spec. +// +// Why: an Agent is the "configured invoker" — model tier + base +// system prompt + a palette of capabilities (skills, sub-agents, +// low-level tools) it may exercise during a run. Where a Skill is a +// reusable parameterised callable (a library function), an Agent is +// the actor with a persistent persona that calls those skills. The +// struct is flat — every field lives on its own column on the +// agents table; JSON columns are used only for variable-length +// collections (palette lists, tags, etc.). +// +// What: identity + persona + execution caps + palette + triggers + +// personalization + UX, all on one struct. Several field families +// (Palette, Triggers, Personalization) are persisted now but NOT +// exercised until later phases — they exist so the schema is stable +// and future phases can light up behaviour without DB migrations. +// +// Test: see pkg/logic/agents/storage_round_trip_test.go for +// Save/Get/GetByName/List/Delete coverage. +type Agent struct { + // Identity + ID string // UUID + Name string // unique per OwnerID; human-friendly identifier + Description string + OwnerID string // Discord member ID + AuthoredBy string // Discord member ID; usually == OwnerID + Version int // monotonic, for future versioning + CreatedAt time.Time + UpdatedAt time.Time + + // Extends names the parent agent this agent inherits from. Only used + // during builtin loading — the loader resolves extends references and + // merges fields before persisting. The resolved agent is a standalone + // entity; Extends is NOT persisted in the database. Only single-level + // extends is supported (no chains). + Extends string + + // SystemPromptPrepend, when non-empty, is prepended to the system + // prompt (with a trailing newline separator). Used by the extends + // mechanism so a child agent can prepend persona instructions to the + // parent's full system prompt without duplicating it. Like Extends, + // this is resolved at load time and NOT persisted — the final + // SystemPrompt on the persisted Agent already has the prepend applied. + SystemPromptPrepend string + + // Persona / execution spec + ModelTier string // "fast" | "standard" | "thinking" | provider/model + SystemPrompt string // base persona prompt (Phase 5 layers personalization on top) + MaxIterations int // 0 → use convar default at execution time + MaxToolCalls int // 0 → use convar default at execution time + MaxRuntime time.Duration // stored as MaxRuntimeNs int64 in GORM (avoid duration-driver flakiness) + ExecutionLane string // lane name; empty = default at execution time + EncryptionEnabled bool // Phase 1 stores the flag; envelope-encryption bridge wires in a later phase + + // Run-critic (two-tier timeout). When CriticEnabled is false (the + // default) MaxRuntime is the hard kill, exactly as before. When true, + // MaxRuntime becomes a SOFT trigger: at MaxRuntime the run-critic + // activates and periodically reviews the run; the hard backstop (the + // absolute kill) is MaxRuntime × the multiplier. CriticBackstopMultiplier + // of 0 means "use the convar default" (agents.critic.backstop_multiplier_default, + // default 6×). See pkg/logic/agentcritic. + CriticEnabled bool + CriticBackstopMultiplier float64 + + // Palette — what this Agent may invoke (Phase 2 reads these). + // Stored as JSON arrays; not exercised by Phase 1 CRUD. + SkillPalette []string // skill IDs/names + SubAgentPalette []string // agent IDs/names + LowLevelTools []string // skilltools registry names + + // Personalization (Phase 5 reads these). Each layer name maps to + // a registered PersonalizationProvider that returns text appended + // to SystemPrompt at run time. Empty list = base prompt only. + PersonalizationSources []string + + // Triggers — persisted now, NOT dispatched by Phase 1. + // + // Schedule: cron expression or "daily"/"weekly" shorthand. Empty + // = on-demand only. NextRunAt + LastScheduledRunAt are scheduler + // bookkeeping for Phase 3's per-Agent scheduler. + Schedule string + NextRunAt *time.Time + LastScheduledRunAt *time.Time + + // Webhook trigger metadata. WebhookSecret empty = inbound + // webhooks disabled. WebhookSignatureRequired defaults true at + // save time (see Skill's lesson: don't store a GORM default on a + // bool where false is a legitimate explicit value — application + // layer is the source of truth). + WebhookSecret string + WebhookSignatureRequired bool + WebhookIPAllowlist []string // CIDR strings; stored as JSON array + + // Chatbot trigger metadata. ChatbotChannelFilter names a filter + // registered in pkg/logic/skills' ChannelFilterRegistry — when + // the migrated chatbot dispatches via this Agent, the filter + // gates which channels it listens in. + ChatbotChannelFilter string + + // UX + // + // DefaultEmoji is an optional identity emoji for this agent. + // Used as the __start__ fallback and forwarded to the invoking + // Discord message when a parent calls this agent via agent_invoke. + DefaultEmoji string + + // StateReactEmoji maps tool names (and reserved keys "__start__", + // "__end__", "__error__") to Discord emoji that the executor + // reacts with as the run progresses. Empty map = no reactions. + StateReactEmoji map[string]string + + // Tags is a free-form set of short labels for organisation + + // discovery on the agents list page (Phase 1 admin commands + + // future web UI). + Tags []string + + // Phases defines a multi-phase pipeline for this agent. When + // non-empty, the executor runs agentexec's sequential phase runner + // instead of the single agent loop. Empty = single-loop agent. + // + // Phases IS persisted (JSON struct-slice column `phases` on + // gormAgent). It used to be transient — "TOML is the only source of + // truth" — but every production dispatch path resolves the agent from + // the DB, where the dropped Phases meant research / deepresearch + // silently degraded to a single-loop run (the executor's + // `len(a.Phases) > 0` pipeline branch was dead). The builtin loader + // still seeds phases from YAML; persisting them is what makes the + // pipeline branch fire for DB-loaded agents. + Phases []AgentPhase +} + +// AgentPhase describes one stage of a multi-phase pipeline in an +// agent definition. Executed directly by agentexec's phase runner +// (pipeline.go) — there is no intermediate execution-spec struct. +// +// What: name + prompt template + model/iteration overrides + tool +// list + optional/fallback flags + IsRunFunc indicator. +// +// Test: see builtin_loader_test.go for YAML round-trip coverage. +type AgentPhase struct { + // Name identifies the phase (e.g., "scout", "plan", "investigate"). + Name string + + // SystemPrompt for this phase. Supports template variables: + // {{.Query}} for the original query, and {{.}} for + // prior phase outputs (e.g., {{.scout}}, {{.plan}}). + SystemPrompt string + + // ModelTier overrides the agent's ModelTier for this phase. + // Empty = use agent default. + ModelTier string + + // MaxIter overrides the agent's MaxIterations for this phase. + // 0 = use agent default. + MaxIter int + + // Tools are tool names for this phase only. These are resolved + // from the agent's low-level tools + palette at execution time. + Tools []string + + // Optional means errors in this phase don't abort the pipeline. + Optional bool + + // FallbackMessage is used when an optional phase fails. + // Default: "(Phase encountered an error)" + FallbackMessage string + + // IsRunFunc indicates this phase is a bare LLM call (no tool + // loop). When true, the executor makes a single model.Complete + // call instead of running the full agent loop. + IsRunFunc bool +} diff --git a/persona/builtin_loader.go b/persona/builtin_loader.go new file mode 100644 index 0000000..ae31f22 --- /dev/null +++ b/persona/builtin_loader.go @@ -0,0 +1,570 @@ +package persona + +// Phase 6 — Builtin Agent loader. +// +// Why: Phase 1-5 introduced the Agent noun, runtime, triggers, +// CommandBinding, and ChatBot bridge — but every Agent in production +// was either (a) a wrapper auto-migrated from a triggered Skill, or +// (b) admin-created via `.agent new`. There were no SHIPPED Agents +// authored as builtins. Phase 6 adds an idempotent boot-time loader so +// the repo can ship canonical Agent definitions (alongside the +// existing skills//skill.yml builtins) without manual admin +// creation per deploy. +// +// What: scans `/agents/*/agent.yml`, decodes each YAML +// into an Agent, and upserts via Storage.SaveAgent under the deterministic +// system owner ID "builtin". Skill-palette entries are validated AT LOAD +// TIME against the live skills storage; missing skills warn but do not +// fail the load (the skill might arrive later via a different code +// path, and runtime resolution happens at invocation time anyway). +// +// Bypass note (v3 lesson, mirrored): like skills.LoadBuiltins, this +// loader writes via Storage.SaveAgent directly. There is no agents +// equivalent of SaveUserSkill's save-time gates today (Phase 1-5 don't +// have authoring requirements on agents), but if such gates appear in +// future phases, this loader MUST keep bypassing them — builtins are +// trusted infrastructure. +// +// Test: pkg/logic/agents/builtin_loader_test.go covers happy path, +// idempotent re-load, missing-skill warn capture, and malformed YAML +// surfaced as a per-bundle warning (not a fatal error). + +import ( + "context" + "errors" + "fmt" + "io/fs" + "log/slog" + "path" + "strings" + "time" + + "github.com/google/uuid" + "gopkg.in/yaml.v3" +) + +// BuiltinAgentOwnerID is the deterministic system owner ID used for +// every Agent created by LoadBuiltinAgents. Chosen as a non-empty +// string so the (owner_id, name) unique index distinguishes builtins +// from any user-authored Agent (Discord member IDs are numeric, so +// "builtin" cannot collide). The skills builtin loader uses owner_id="" +// instead; the two systems are independent storage scopes — there's +// no need for consistency here. +const BuiltinAgentOwnerID = "builtin" + +// SkillExistenceChecker is the narrow surface LoadBuiltinAgents needs +// to validate skill_palette entries at load time. Production wires +// skills.Storage which already exposes ListByName for non-owner-scoped +// lookups. nil means "skip palette validation" (tests that don't care). +// +// Why a separate narrow interface (vs importing skills.Storage): +// agents already transitively depends on skills via migrate_from_skills, +// but the loader only needs "does a skill with this name exist +// somewhere?" — a single Boolean. Keeping the interface narrow makes +// the loader testable without a full skills storage stub. +type SkillExistenceChecker interface { + // SkillExistsByName reports whether at least one skill row has the + // given name across any owner (builtins live under owner_id=""; + // users own their own rows; the loader's validation just wants + // "does ANY row exist with this name?"). + SkillExistsByName(ctx context.Context, name string) (bool, error) +} + +// LoadBuiltinAgents discovers and seeds builtin Agents from `builtinsDir`. +// `builtinsDir` is the root that contains an `agents/` subdirectory; +// per-agent YAML lives at `agents//agent.yml`. Returns the count +// of agents seeded or updated (skipped rows do not contribute to the +// count). Returns nil error when the agents/ directory is absent — a +// deployment without any builtin agents is valid; the loader is then a +// no-op. +// +// Idempotency contract: existing Agent rows (matched by (owner_id="builtin", +// name)) are UPDATED to the freshly-parsed YAML on each boot. ID + +// CreatedAt are preserved; UpdatedAt is refreshed. User clones of a +// builtin Agent (different owner_id, same name) are NEVER touched — +// the loader only writes to (owner_id="builtin", name) rows. +// +// `skillChecker` may be nil; when non-nil, each SkillPalette entry is +// looked up and a WARN log emitted (with the agent + missing skill +// name) for absent references. The Agent row is still seeded with the +// palette intact — runtime resolution at invocation time is the +// authoritative gate. +func LoadBuiltinAgents(ctx context.Context, store Storage, builtinsDir fs.FS, skillChecker SkillExistenceChecker) (int, error) { + if store == nil { + return 0, errors.New("agents.LoadBuiltinAgents: nil store") + } + if builtinsDir == nil { + return 0, errors.New("agents.LoadBuiltinAgents: nil builtinsDir FS") + } + entries, err := fs.ReadDir(builtinsDir, "agents") + if err != nil { + // Missing agents/ directory is benign — a deployment may not + // ship any builtins. Other errors propagate so a permission / + // IO problem surfaces loudly. + if errors.Is(err, fs.ErrNotExist) { + slog.Info("agents: no builtin agents directory", "path", "agents") + return 0, nil + } + return 0, fmt.Errorf("agents: read agents dir: %w", err) + } + + // Phase 1: parse all agent manifests into a map keyed by name. + // The map is needed so extends references can be resolved before + // any agent is upserted. + type parsedEntry struct { + agent *Agent + dir string + } + parsed := make(map[string]*parsedEntry) + var parseOrder []string // preserve FS iteration order for deterministic upsert + + var scanned, failed int + for _, entry := range entries { + if !entry.IsDir() { + continue + } + manifestPath := path.Join("agents", entry.Name(), "agent.yml") + data, readErr := fs.ReadFile(builtinsDir, manifestPath) + if readErr != nil { + slog.Debug("agents: skipping (no agent.yml)", "dir", entry.Name(), "error", readErr) + continue + } + scanned++ + ag, parseErr := decodeAgentManifest(data) + if parseErr != nil { + slog.Warn("agents: invalid agent.yml", "dir", entry.Name(), "error", parseErr) + failed++ + continue + } + parsed[ag.Name] = &parsedEntry{agent: ag, dir: entry.Name()} + parseOrder = append(parseOrder, ag.Name) + } + + // Phase 2: resolve extends references. Only single-level is + // supported — chains (A extends B extends C) are rejected. + for _, name := range parseOrder { + pe := parsed[name] + ag := pe.agent + if ag.Extends == "" { + continue + } + parent, ok := parsed[ag.Extends] + if !ok { + slog.Warn("agents: extends references unknown agent", + "agent", ag.Name, "extends", ag.Extends) + failed++ + delete(parsed, name) + continue + } + if parent.agent.Extends != "" { + slog.Warn("agents: extends chain not supported — parent also uses extends", + "agent", ag.Name, "extends", ag.Extends, + "parent_extends", parent.agent.Extends) + failed++ + delete(parsed, name) + continue + } + if ag.Extends == ag.Name { + slog.Warn("agents: agent extends itself", "agent", ag.Name) + failed++ + delete(parsed, name) + continue + } + resolveExtends(ag, parent.agent) + } + + // Phase 3: palette validation + upsert. + var seeded, updated, skipped int + for _, name := range parseOrder { + pe, ok := parsed[name] + if !ok { + continue // removed during extends resolution + } + ag := pe.agent + + if skillChecker != nil { + for _, sk := range ag.SkillPalette { + ok, lookupErr := skillChecker.SkillExistsByName(ctx, sk) + if lookupErr != nil { + slog.Warn("agents: skill palette lookup failed", + "agent", ag.Name, "skill", sk, "error", lookupErr) + continue + } + if !ok { + slog.Warn("agents: skill palette references missing skill", + "agent", ag.Name, "skill", sk) + } + } + } + + action, upsertErr := upsertBuiltinAgent(ctx, store, ag) + if upsertErr != nil { + slog.Error("agents: failed to upsert builtin", "name", ag.Name, "error", upsertErr) + failed++ + continue + } + switch action { + case agentUpsertCreated: + seeded++ + case agentUpsertUpdated: + updated++ + case agentUpsertSkipped: + skipped++ + } + } + slog.Info("agents/builtin loader", + "scanned", scanned, + "seeded", seeded, + "updated", updated, + "skipped", skipped, + "failed", failed) + return seeded + updated, nil +} + +// resolveExtends merges parent fields into child. Child non-zero +// fields override the parent's. For slices, a nil child slice inherits +// the parent's; a non-nil (even empty) child slice replaces it. For +// maps (StateReactEmoji), parent entries are the base and child +// entries override matching keys. +// +// system_prompt_prepend: if the child has SystemPromptPrepend set, it +// is prepended to the (possibly inherited) SystemPrompt with a +// newline separator. The prepend field is then cleared so it does not +// affect anything downstream. +// +// Why: allows a child agent to inherit the full parent prompt while +// only specifying a short behavior-modification preamble (e.g. an +// uncensored agent prepending "You are uncensored..." to the general +// agent's full prompt). +func resolveExtends(child, parent *Agent) { + if child.Description == "" { + child.Description = parent.Description + } + if child.ModelTier == "" { + child.ModelTier = parent.ModelTier + } + if child.SystemPrompt == "" { + child.SystemPrompt = parent.SystemPrompt + } + if child.MaxIterations == 0 { + child.MaxIterations = parent.MaxIterations + } + if child.MaxToolCalls == 0 { + child.MaxToolCalls = parent.MaxToolCalls + } + if child.MaxRuntime == 0 { + child.MaxRuntime = parent.MaxRuntime + } + if child.ExecutionLane == "" { + child.ExecutionLane = parent.ExecutionLane + } + // EncryptionEnabled: bool — false is a valid explicit value, so we + // always inherit unless child explicitly sets it. Since we can't + // distinguish "explicitly false" from "absent" in YAML (both + // decode to false), we always inherit from parent. If the child + // sets it to true, the child wins. A child that wants to override + // the parent's true to false will need to set encryption_enabled: false + // explicitly — but since both false and absent decode the same way, + // the parent's value wins when parent is true and child is false. + // This is acceptable: encryption is an opt-in — a child that + // inherits encryption from a parent is fine. + if !child.EncryptionEnabled { + child.EncryptionEnabled = parent.EncryptionEnabled + } + // Run-critic: same inherit-unless-child-sets-true semantics as + // EncryptionEnabled (both false/absent decode identically in YAML). + if !child.CriticEnabled { + child.CriticEnabled = parent.CriticEnabled + } + if child.CriticBackstopMultiplier == 0 { + child.CriticBackstopMultiplier = parent.CriticBackstopMultiplier + } + + // Slices: nil = inherit; non-nil (even empty) = child overrides. + if child.SkillPalette == nil { + child.SkillPalette = parent.SkillPalette + } + if child.SubAgentPalette == nil { + child.SubAgentPalette = parent.SubAgentPalette + } + if child.LowLevelTools == nil { + child.LowLevelTools = parent.LowLevelTools + } + if child.PersonalizationSources == nil { + child.PersonalizationSources = parent.PersonalizationSources + } + if child.Tags == nil { + child.Tags = parent.Tags + } + if child.WebhookIPAllowlist == nil { + child.WebhookIPAllowlist = parent.WebhookIPAllowlist + } + if child.Phases == nil { + child.Phases = parent.Phases + } + + // Triggers (Schedule, ChatbotChannelFilter, WebhookSecret, …) are + // deliberately NOT inherited. A trigger is an ACTIVATION decision — + // "this agent fires on a schedule" / "this agent is a chatbot tool in + // these channels" — and silently inheriting it from a parent persona + // is a behavioural surprise: `uncensored extends general` would inherit + // general's `chatbot_channel_filter: "none"` (match-every-channel) and + // surface the unfiltered model as a direct chatbot tool everywhere the + // instant agents.triggers.enabled flips on. A child that wants a trigger + // must declare it explicitly. (Persona, caps, palette, and tools are + // inherited above — those are capability, not activation.) + + // DefaultEmoji: child wins if set; otherwise inherit. + if child.DefaultEmoji == "" { + child.DefaultEmoji = parent.DefaultEmoji + } + + // Maps: merge — parent is the base, child entries override. + if child.StateReactEmoji == nil && parent.StateReactEmoji != nil { + child.StateReactEmoji = make(map[string]string, len(parent.StateReactEmoji)) + for k, v := range parent.StateReactEmoji { + child.StateReactEmoji[k] = v + } + } else if parent.StateReactEmoji != nil { + merged := make(map[string]string, len(parent.StateReactEmoji)+len(child.StateReactEmoji)) + for k, v := range parent.StateReactEmoji { + merged[k] = v + } + for k, v := range child.StateReactEmoji { + merged[k] = v + } + child.StateReactEmoji = merged + } + + // SystemPromptPrepend: prepend to the (now resolved) SystemPrompt. + if child.SystemPromptPrepend != "" { + child.SystemPrompt = child.SystemPromptPrepend + "\n\n" + child.SystemPrompt + child.SystemPromptPrepend = "" // consumed + } + + // Clear Extends — the resolution is complete, the persisted agent + // is standalone. + child.Extends = "" +} + +// agentUpsertAction reports what upsertBuiltinAgent did. Exported only +// to the test in this package; the loader's public surface returns a +// count, not a per-row action. +type agentUpsertAction int + +const ( + agentUpsertCreated agentUpsertAction = iota + agentUpsertUpdated + agentUpsertSkipped // reserved; current loader never returns this — every parse-OK row is upserted +) + +// upsertBuiltinAgent looks up an existing (BuiltinAgentOwnerID, name) +// row. If absent, inserts a new row with a freshly-minted UUID. +// Otherwise updates the existing row in place, preserving ID + CreatedAt. +// +// Why not version-skip like skills.upsertBuiltin: the Agent struct has +// a Version int field but it's a monotonic counter, not a semver +// string for change detection. Agent YAML doesn't carry a "version" +// at the wire shape; every boot writes the latest YAML content, +// trusting the YAML file in-repo IS the source of truth. The Agent's +// internal Version int auto-increments on each loader pass so admin +// inspection (`.agent show`) reveals "how many times has the loader +// touched this row". +func upsertBuiltinAgent(ctx context.Context, store Storage, fresh *Agent) (agentUpsertAction, error) { + existing, err := store.GetAgentByName(ctx, BuiltinAgentOwnerID, fresh.Name) + if err != nil && !errors.Is(err, ErrNotFound) { + return agentUpsertCreated, fmt.Errorf("lookup builtin agent %q: %w", fresh.Name, err) + } + if errors.Is(err, ErrNotFound) { + fresh.ID = uuid.New().String() + fresh.OwnerID = BuiltinAgentOwnerID + fresh.AuthoredBy = BuiltinAgentOwnerID + if fresh.Version == 0 { + fresh.Version = 1 + } + now := time.Now() + fresh.CreatedAt = now + fresh.UpdatedAt = now + if saveErr := store.SaveAgent(ctx, fresh); saveErr != nil { + return agentUpsertCreated, saveErr + } + slog.Info("agents: created builtin", "name", fresh.Name, "id", fresh.ID) + return agentUpsertCreated, nil + } + // Update in place. Preserve ID, OwnerID, AuthoredBy, CreatedAt. + // Bump Version so admins can see "the loader has touched this N + // times" — useful when investigating a builtin that was + // hand-edited via the future web UI and unexpectedly reverted on + // next boot. + fresh.ID = existing.ID + fresh.OwnerID = BuiltinAgentOwnerID + fresh.AuthoredBy = BuiltinAgentOwnerID + fresh.Version = existing.Version + 1 + fresh.CreatedAt = existing.CreatedAt + fresh.UpdatedAt = time.Now() + // Carry forward operator/scheduler-owned fields that the manifest + // never sets (decodeAgentManifest leaves these zero by design — a + // secret in-repo would be a credential leak). Without this, every + // boot CLOBBERS an operator-armed webhook secret + signature flag + // back to empty/false and nukes the scheduler's next-fire cursor, so + // a scheduled or webhook-armed builtin silently breaks on each deploy. + fresh.WebhookSecret = existing.WebhookSecret + fresh.WebhookSignatureRequired = existing.WebhookSignatureRequired + fresh.NextRunAt = existing.NextRunAt + fresh.LastScheduledRunAt = existing.LastScheduledRunAt + if saveErr := store.SaveAgent(ctx, fresh); saveErr != nil { + return agentUpsertUpdated, saveErr + } + slog.Info("agents: updated builtin", + "name", fresh.Name, "id", fresh.ID, "version", fresh.Version) + return agentUpsertUpdated, nil +} + +// builtinAgentManifest is the YAML wire format for agents//agent.yml. +// The schema is intentionally a SUBSET of the Agent struct — future +// fields can be added without breaking existing manifests so long as +// we keep KnownFields(true) decoding (so a typo on a key surfaces as +// an error rather than silently dropping data). +// +// See pkg/logic/agents/CLAUDE.md for the schema reference. +type builtinAgentManifest struct { + Name string `yaml:"name"` + Description string `yaml:"description"` + ModelTier string `yaml:"model_tier"` + SystemPrompt string `yaml:"system_prompt"` + SystemPromptPrepend string `yaml:"system_prompt_prepend"` + MaxIterations int `yaml:"max_iterations"` + MaxToolCalls int `yaml:"max_tool_calls"` + MaxRuntimeSeconds int `yaml:"max_runtime_seconds"` + ExecutionLane string `yaml:"execution_lane"` + EncryptionEnabled bool `yaml:"encryption_enabled"` + + // Run-critic two-tier timeout. CriticEnabled flips MaxRuntime from a + // hard kill into a soft trigger; CriticBackstopMultiplier (0 => convar + // default 6×) sets the hard backstop = MaxRuntime × multiplier. + CriticEnabled bool `yaml:"critic_enabled"` + CriticBackstopMultiplier float64 `yaml:"critic_backstop_multiplier"` + + // Extends names a parent agent whose fields are inherited. The + // child's non-zero fields override the parent; nil/empty slices + // inherit the parent's. Maps (state_react) are merged — child + // entries override parent entries with the same key. Only single- + // level extends is supported (no chains). + Extends string `yaml:"extends"` + + SkillPalette []string `yaml:"skill_palette"` + SubAgentPalette []string `yaml:"sub_agent_palette"` + LowLevelTools []string `yaml:"low_level_tools"` + + PersonalizationSources []string `yaml:"personalization_sources"` + + // Triggers — builtin agents typically don't ship with triggers + // (admins flip these on per-deployment), but the keys are accepted + // so a sufficiently sophisticated builtin (e.g. a scheduled "weekly + // digest" agent) can ship triggers in-repo. Default empty. + Schedule string `yaml:"schedule"` + WebhookIPAllowlist []string `yaml:"webhook_ip_allowlist"` + ChatbotChannelFilter string `yaml:"chatbot_channel_filter"` + + DefaultEmoji string `yaml:"default_emoji"` + StateReact map[string]string `yaml:"state_react"` + Tags []string `yaml:"tags"` + + // Pipeline phases — when non-empty, the executor runs the + // sequential phase runner instead of the single agent loop. + Phases []builtinAgentPhaseManifest `yaml:"phases"` +} + +// builtinAgentPhaseManifest is the YAML wire format for a single +// phases list entry in agents//agent.yml. Maps 1:1 to +// AgentPhase at decode time. +type builtinAgentPhaseManifest struct { + Name string `yaml:"name"` + SystemPrompt string `yaml:"system_prompt"` + ModelTier string `yaml:"model_tier"` + MaxIter int `yaml:"max_iter"` + Tools []string `yaml:"tools"` + Optional bool `yaml:"optional"` + FallbackMessage string `yaml:"fallback_message"` + IsRunFunc bool `yaml:"is_run_func"` +} + +// decodeAgentManifest parses an agent.yml bundle into a domain Agent. +// Uses KnownFields(true) so a typo'd key surfaces as a parse error +// rather than silently dropping the value. +// +// What this method does NOT set: +// - ID (loader mints UUID on insert / preserves existing on update) +// - OwnerID + AuthoredBy (loader sets to BuiltinAgentOwnerID) +// - Version (loader increments on update) +// - CreatedAt + UpdatedAt (loader stamps) +// - WebhookSecret (operator generates via admin tooling at deploy +// time — shipping a secret in-repo would be a credential leak) +// - NextRunAt + LastScheduledRunAt (scheduler bookkeeping; nil at +// load time, populated on first scheduled fire) +// - WebhookSignatureRequired (application-layer default applies on +// first save; a `default:true` GORM tag would substitute on every +// write — see the v8 lesson on this exact trap) +func decodeAgentManifest(data []byte) (*Agent, error) { + var m builtinAgentManifest + dec := yaml.NewDecoder(strings.NewReader(string(data))) + dec.KnownFields(true) + if err := dec.Decode(&m); err != nil { + return nil, fmt.Errorf("decode agent.yml: %w", err) + } + if strings.TrimSpace(m.Name) == "" { + return nil, errors.New("agent.yml: missing required field 'name'") + } + // system_prompt is required UNLESS the agent uses extends (the parent + // will supply it) or system_prompt_prepend (the prepend will be + // combined with the parent's system_prompt after extends resolution). + if strings.TrimSpace(m.SystemPrompt) == "" && strings.TrimSpace(m.Extends) == "" && strings.TrimSpace(m.SystemPromptPrepend) == "" { + return nil, errors.New("agent.yml: missing required field 'system_prompt'") + } + + // Convert YAML phase manifests to domain AgentPhase structs. + var phases []AgentPhase + for _, pm := range m.Phases { + if strings.TrimSpace(pm.Name) == "" { + return nil, errors.New("agent.yml: phase missing required field 'name'") + } + phases = append(phases, AgentPhase{ + Name: strings.TrimSpace(pm.Name), + SystemPrompt: pm.SystemPrompt, + ModelTier: strings.TrimSpace(pm.ModelTier), + MaxIter: pm.MaxIter, + Tools: pm.Tools, + Optional: pm.Optional, + FallbackMessage: pm.FallbackMessage, + IsRunFunc: pm.IsRunFunc, + }) + } + + ag := &Agent{ + Name: strings.TrimSpace(m.Name), + Description: m.Description, + Extends: strings.TrimSpace(m.Extends), + SystemPromptPrepend: m.SystemPromptPrepend, + ModelTier: strings.TrimSpace(m.ModelTier), + SystemPrompt: m.SystemPrompt, + MaxIterations: m.MaxIterations, + MaxToolCalls: m.MaxToolCalls, + MaxRuntime: time.Duration(m.MaxRuntimeSeconds) * time.Second, + ExecutionLane: strings.TrimSpace(m.ExecutionLane), + EncryptionEnabled: m.EncryptionEnabled, + CriticEnabled: m.CriticEnabled, + CriticBackstopMultiplier: m.CriticBackstopMultiplier, + SkillPalette: m.SkillPalette, + SubAgentPalette: m.SubAgentPalette, + LowLevelTools: m.LowLevelTools, + PersonalizationSources: m.PersonalizationSources, + Schedule: strings.TrimSpace(m.Schedule), + WebhookIPAllowlist: m.WebhookIPAllowlist, + ChatbotChannelFilter: strings.TrimSpace(m.ChatbotChannelFilter), + DefaultEmoji: m.DefaultEmoji, + StateReactEmoji: m.StateReact, + Tags: m.Tags, + Phases: phases, + } + return ag, nil +} diff --git a/persona/memory.go b/persona/memory.go new file mode 100644 index 0000000..cbbf840 --- /dev/null +++ b/persona/memory.go @@ -0,0 +1,120 @@ +package persona + +import ( + "context" + "sort" + "sync" + "time" +) + +// Memory is a zero-dependency in-process Storage for agent personas — a light +// host (or tests) gets persona persistence with no DB. Mort keeps its +// GORM/MySQL Storage; contrib/store adds a durable SQLite one. +type Memory struct { + mu sync.RWMutex + agents map[string]*Agent // by ID +} + +// NewMemory returns an empty in-memory persona Storage. +func NewMemory() *Memory { return &Memory{agents: map[string]*Agent{}} } + +var _ Storage = (*Memory)(nil) + +func (m *Memory) InitializeAgentStorage(context.Context) error { return nil } + +func (m *Memory) SaveAgent(_ context.Context, a *Agent) error { + m.mu.Lock() + defer m.mu.Unlock() + cp := *a + m.agents[a.ID] = &cp + return nil +} + +func (m *Memory) GetAgent(_ context.Context, id string) (*Agent, error) { + m.mu.RLock() + defer m.mu.RUnlock() + a, ok := m.agents[id] + if !ok { + return nil, ErrNotFound + } + cp := *a + return &cp, nil +} + +func (m *Memory) GetAgentByName(_ context.Context, ownerID, name string) (*Agent, error) { + m.mu.RLock() + defer m.mu.RUnlock() + for _, a := range m.agents { + if a.OwnerID == ownerID && a.Name == name { + cp := *a + return &cp, nil + } + } + return nil, ErrNotFound +} + +func (m *Memory) listWhere(keep func(*Agent) bool) []*Agent { + m.mu.RLock() + defer m.mu.RUnlock() + out := make([]*Agent, 0, len(m.agents)) + for _, a := range m.agents { + if keep == nil || keep(a) { + cp := *a + out = append(out, &cp) + } + } + sort.Slice(out, func(i, j int) bool { return out[i].Name < out[j].Name }) + return out +} + +func (m *Memory) ListAgents(_ context.Context, ownerID string) ([]*Agent, error) { + return m.listWhere(func(a *Agent) bool { return a.OwnerID == ownerID }), nil +} + +func (m *Memory) ListAllAgents(context.Context) ([]*Agent, error) { + return m.listWhere(nil), nil +} + +func (m *Memory) DeleteAgent(_ context.Context, id string) error { + m.mu.Lock() + defer m.mu.Unlock() + delete(m.agents, id) + return nil +} + +func (m *Memory) GetAgentByWebhookSecret(_ context.Context, secret string) (*Agent, error) { + if secret == "" { + return nil, ErrNotFound + } + m.mu.RLock() + defer m.mu.RUnlock() + for _, a := range m.agents { + if a.WebhookSecret == secret { + cp := *a + return &cp, nil + } + } + return nil, ErrNotFound +} + +func (m *Memory) ListAgentsByChatbotChannelFilter(context.Context) ([]*Agent, error) { + return m.listWhere(func(a *Agent) bool { return a.ChatbotChannelFilter != "" }), nil +} + +func (m *Memory) ListScheduledAgents(_ context.Context, dueBefore time.Time) ([]*Agent, error) { + return m.listWhere(func(a *Agent) bool { + return a.Schedule != "" && a.NextRunAt != nil && !a.NextRunAt.After(dueBefore) + }), nil +} + +func (m *Memory) MarkAgentScheduledRun(_ context.Context, agentID string, ranAt, nextAt time.Time) error { + m.mu.Lock() + defer m.mu.Unlock() + a, ok := m.agents[agentID] + if !ok { + return ErrNotFound + } + a.LastScheduledRunAt = &ranAt + a.NextRunAt = &nextAt + return nil +} diff --git a/persona/persona_test.go b/persona/persona_test.go new file mode 100644 index 0000000..9434f6a --- /dev/null +++ b/persona/persona_test.go @@ -0,0 +1,45 @@ +package persona + +import ( + "context" + "testing" + "time" +) + +func TestToRunnable(t *testing.T) { + a := &Agent{ + ID: "id1", Name: "helper", SystemPrompt: "be nice", ModelTier: "fast", + MaxIterations: 5, MaxRuntime: 30 * time.Second, + LowLevelTools: []string{"think"}, SkillPalette: []string{"animate"}, + CriticEnabled: true, CriticBackstopMultiplier: 2, + Phases: []AgentPhase{{Name: "p1", ModelTier: "thinking", MaxIter: 3, Tools: []string{"now"}, Optional: true}}, + } + r := a.ToRunnable() + if r.ID != "id1" || r.ModelTier != "fast" || r.MaxIterations != 5 || !r.Critic.Enabled { + t.Fatalf("ToRunnable mapping wrong: %+v", r) + } + if len(r.Phases) != 1 || r.Phases[0].MaxIterations != 3 || !r.Phases[0].Optional { + t.Fatalf("phase mapping wrong: %+v", r.Phases) + } +} + +func TestMemoryStoreRoundTrip(t *testing.T) { + ctx := context.Background() + m := NewMemory() + a := &Agent{ID: "a1", Name: "n", OwnerID: "o1"} + if err := m.SaveAgent(ctx, a); err != nil { + t.Fatal(err) + } + got, err := m.GetAgent(ctx, "a1") + if err != nil || got.Name != "n" { + t.Fatalf("GetAgent: %v %+v", err, got) + } + byName, err := m.GetAgentByName(ctx, "o1", "n") + if err != nil || byName.ID != "a1" { + t.Fatalf("GetAgentByName: %v %+v", err, byName) + } + list, _ := m.ListAgents(ctx, "o1") + if len(list) != 1 { + t.Fatalf("ListAgents = %d", len(list)) + } +} diff --git a/persona/runnable.go b/persona/runnable.go new file mode 100644 index 0000000..7a4bafd --- /dev/null +++ b/persona/runnable.go @@ -0,0 +1,37 @@ +package persona + +import "gitea.stevedudenhoeffer.com/steve/executus/run" + +// ToRunnable lowers an Agent persona into the kernel's run.RunnableAgent DTO — +// the bridge that lets run.Executor run a persona WITHOUT importing this +// battery (the inversion of mort's agentexec.Run(*agents.Agent)). It maps the +// static shape only; per-run personalization, palette resolution, the critic, +// audit, etc. are supplied separately via run.Ports. +func (a *Agent) ToRunnable() run.RunnableAgent { + ra := run.RunnableAgent{ + ID: a.ID, + Name: a.Name, + SystemPrompt: a.SystemPrompt, + ModelTier: a.ModelTier, + MaxIterations: a.MaxIterations, + MaxRuntime: a.MaxRuntime, + LowLevelTools: a.LowLevelTools, + SkillPalette: a.SkillPalette, + SubAgentPalette: a.SubAgentPalette, + Critic: run.CriticConfig{ + Enabled: a.CriticEnabled, + BackstopMultiplier: a.CriticBackstopMultiplier, + }, + } + for _, p := range a.Phases { + ra.Phases = append(ra.Phases, run.Phase{ + Name: p.Name, + SystemPrompt: p.SystemPrompt, + ModelTier: p.ModelTier, + MaxIterations: p.MaxIter, + Tools: p.Tools, + Optional: p.Optional, + }) + } + return ra +} diff --git a/persona/storage.go b/persona/storage.go new file mode 100644 index 0000000..46eb88f --- /dev/null +++ b/persona/storage.go @@ -0,0 +1,115 @@ +package persona + +import ( + "context" + "errors" + "time" +) + +// ErrNotFound is returned when an agent lookup fails. Callers compare +// with errors.Is(err, ErrNotFound). +var ErrNotFound = errors.New("agent not found") + +// Storage is the persistence interface for the agents system. +// +// Why: tests substitute fake implementations; production wires +// through pkg/logic/storage's Grand Storage which embeds this +// interface. Mirrors the three-layer pattern in +// pkg/logic/storage/CLAUDE.md (domain → GORM → DB). +// +// What: Phase 1 CRUD plus Phase 3 trigger queries +// (ListDueScheduled, GetAgentByWebhookSecret, +// ListAgentsByChatbotChannelFilter, MarkScheduledRun). Trigger +// queries are read by the agentsched runner, webhook router, and +// chatbot tool provider; all are gated behind the +// agents.triggers.enabled convar so old skill-driven paths keep +// running until the convar flips. +// +// Test: see storage_round_trip_test.go for round-trip coverage. +type Storage interface { + // (Mort's Discord command-binding CRUD — the CommandBindingStorage + // embedding — stays a host concern and is NOT part of the executus + // persona Storage seam.) + + // InitializeAgentStorage prepares storage (e.g. AutoMigrate) + // and is idempotent. Called from the grand storage's + // InitializeAll path. + InitializeAgentStorage(ctx context.Context) error + + // SaveAgent creates or updates an Agent by ID. ID must be + // non-empty (Phase 1 admin commands mint a UUID). + SaveAgent(ctx context.Context, a *Agent) error + + // GetAgent returns the agent with the given ID, or ErrNotFound. + GetAgent(ctx context.Context, id string) (*Agent, error) + + // GetAgentByName resolves (owner_id, name) → agent. ownerID + // must match exactly (Phase 1 has no shared/public visibility + // yet; every agent is owned). + GetAgentByName(ctx context.Context, ownerID, name string) (*Agent, error) + + // ListAgents returns every agent owned by the given member ID, + // sorted by Name ASC. + ListAgents(ctx context.Context, ownerID string) ([]*Agent, error) + + // ListAllAgents returns every agent across all owners, sorted by + // (OwnerID ASC, Name ASC) so builtin rows (OwnerID="builtin") + // group together, then numeric Discord-ID owners in lexical order, + // then chatbot-shadow rows whose OwnerID is the chatbot owner's + // Discord ID but whose Name carries the "chatbot:" prefix. + // + // Why: Phase 1 admin commands ran owner-scoped (a steve-owned + // agent list shows ONLY steve's rows), which hid builtin and + // shadow Agents from the admin view. `.agent list` for admins now + // uses this method to surface every row. Non-admin invocations + // (or `.agent list --mine`) keep using ListAgents. + // + // Storage MAY back this with a single full-table scan — admin + // row counts are small (dozens to low hundreds), so no need for + // pagination at this phase. + ListAllAgents(ctx context.Context) ([]*Agent, error) + + // DeleteAgent removes an agent by ID. Idempotent — deleting a + // missing row returns nil. + DeleteAgent(ctx context.Context, id string) error + + // GetAgentByWebhookSecret resolves a posted /webhooks/ URL + // to the matching agent. Returns ErrNotFound when no agent has + // the secret. Phase 3 webhook router consults this AFTER the + // existing Skill lookup falls through, but only when + // agents.triggers.enabled is true. + // + // Empty secret is rejected with ErrNotFound (empty WebhookSecret + // rows are NOT webhook-enabled — the application layer guards + // this, the lookup defends against accidental match). + GetAgentByWebhookSecret(ctx context.Context, secret string) (*Agent, error) + + // ListAgentsByChatbotChannelFilter returns every agent with a + // non-empty ChatbotChannelFilter. Phase 3 chatbot tool provider + // uses this on every chatbot turn to assemble the per-channel + // tool list (gated by agents.triggers.enabled). The result is + // not channel-filtered here — the provider applies the channel + // filter predicate (registered in skills.ChannelFilterRegistry) + // to each row. + // + // Why no channel filter at the storage layer: the filter is a + // runtime predicate (e.g. dm_only depends on the live Discord + // channel kind cache), not a static column we can index on. + ListAgentsByChatbotChannelFilter(ctx context.Context) ([]*Agent, error) + + // ListScheduledAgents returns every agent with a non-empty + // Schedule whose NextRunAt is at or before `dueBefore`. Result + // is ordered by NextRunAt ASC so the scheduler runner can drain + // in oldest-due-first order. Mirrors skills.Storage.ListDueScheduled. + // + // Phase 3 scheduler reads this on every tick when + // agents.triggers.enabled is true. The (Schedule, NextRunAt) + // composite index backs the query — see gorm tags on gormAgent. + ListScheduledAgents(ctx context.Context, dueBefore time.Time) ([]*Agent, error) + + // MarkAgentScheduledRun atomically updates LastScheduledRunAt + // and NextRunAt for the given agent. Called by the agentsched + // runner after each scheduled invocation. Mirrors + // skills.Storage.MarkScheduledRun semantics. + MarkAgentScheduledRun(ctx context.Context, agentID string, ranAt, nextAt time.Time) error +}