diff --git a/docs/packages/2-feature/selflearn.md b/docs/packages/2-feature/selflearn.md index 2a2a3733..f0d33205 100644 --- a/docs/packages/2-feature/selflearn.md +++ b/docs/packages/2-feature/selflearn.md @@ -5,8 +5,10 @@ layer: feature # selflearn -Runs San's background self-learning pass: on cadence, a restricted fork reviews -recent conversation state and writes durable memory or agent-created skills. +Runs San's background self-learning pass: after a turn, a restricted fork +reviews the just-completed conversation and writes durable memory or +agent-created skills. Triggering is model-decided — the main agent calls the +`Evolve` tool when a turn is worth learning from. ## Purpose @@ -17,8 +19,43 @@ the Bubble Tea model and exposes concrete types for the pieces the app needs. This package deliberately writes only to San-managed durable state: project memory under the encoded project configuration directory and skills marked with -agent-created provenance. User-created skills remain read-only unless the -setting explicitly opts into updates. +agent-created provenance. User-created skills are read-only to the reviewer at +every setting. + +## Triggers & flow + +Triggering is entirely model-decided — there is no cadence. When self-learning +is active the main agent is given an `Evolve` tool; the per-arm settings are +permission / store config only (skill create/update/delete gates, memory +enable + cap + path, a shared strategy override). + +End-to-end when the model calls `Evolve`: + +1. The call is auto-permitted (`Evolve` is a safe tool) and returns an ack; the + model keeps working. `model.OnToolResult` sets `evolveRequestedThisTurn`. +2. At `OnTurnEnd`, `Reviewer.Observe(result, skillUsed, evolveRequested)` fires a + review over every enabled arm — memory if enabled, and skills scoped by skill + use (skill-used → update/delete, skill-free → create) — bounded by the + permission gates. +3. A background fork (`RunReview`) reflects on the turn snapshot with only + `skill_manage` + `memory_write`, then writes memory and/or a skill, or does + nothing. The status bar shows `evolving… → evolved · N changes`. + +The `Evolve` tool's schema is built by `tool/evolve.Schema` from the enabled +capabilities (`evolve.Capabilities`) so the model is only invited to flag +learnings the review can act on — memory off never mentions memory — and is +injected through the toolset's generic `ExtraTools` hook. The writes affect +future sessions, not the current turn; at most one review runs at a time. +Capability changes are reconciled at the next turn start: `ensureAgentSession` +records what the toolset was built with (`agentEvolveCaps`) and rebuilds the +agent on drift — covering /evolve saves and external settings edits with one +mechanism. `SAN_DISABLE_SELF_LEARN=1` is a hard runtime switch: it suppresses +both the reviewer and the `Evolve` tool regardless of layered settings. + +The skills JSON retains an explicit `enabled` compatibility marker. Legacy +empty `skills` objects came from `enabled:false` being omitted and migrate to +all three actions denied; current saves always emit the marker so permissive +permission sets round-trip without ambiguity. ## Contract @@ -30,20 +67,26 @@ need. package selflearn type Config struct { - Memory Arm - Skills Arm - Perms ActionPermissions + MemoryEnabled bool MemoryMaxChars int + MemoryPath string + Skills SkillPermissions + Strategy string // learning-strategy override (empty ⇒ built-in) } func (c Config) Enabled() bool func ResolveSettings(s setting.SelfLearnSettings) (Config, error) -type Arm struct { - Enabled bool - Interval int +// SkillPermissions appears at three altitudes with one meaning: the configured +// gates (Config.Skills), the per-pass scope the trigger derives from them, and +// the SkillManager's hard floor at dispatch. +type SkillPermissions struct { + AllowCreate, AllowUpdate, AllowDelete bool } +func (p SkillPermissions) Any() bool +func AllowAllSkillActions() SkillPermissions + type ReviewKind uint8 const ( @@ -54,28 +97,30 @@ const ( func (k ReviewKind) Has(x ReviewKind) bool func (k ReviewKind) String() string -type ReviewFunc func(kinds ReviewKind, snapshot []core.Message) +type ReviewFunc func(kinds ReviewKind, skillPerms SkillPermissions, snapshot []core.Message) type Reviewer struct { /* unexported */ } func New(cfg Config, review ReviewFunc) *Reviewer -func (r *Reviewer) SeedTurns(priorUserTurns int) -func (r *Reviewer) Observe(result core.Result) +func (r *Reviewer) Observe(result core.Result, skillUsed, evolveRequested bool) + +func DefaultStrategy() string // built-in guidance the /evolve Strategy editor seeds with type ForkConfig struct { - LLM core.LLM - System *system.System - CWD string - Memory *MemoryStore - Skills *SkillManager - OnEvent func(core.Event) + LLM core.LLM + System *system.System + CWD string + Memory *MemoryStore + Skills *SkillManager // its Perms() also scope the review prompt + Strategy string + OnEvent func(core.Event) } func RunReview(ctx context.Context, fc ForkConfig, kinds ReviewKind, snapshot []core.Message) (string, error) type MemoryStore struct { /* unexported */ } -func NewMemoryStore(cwd string, maxFile int) *MemoryStore +func NewMemoryStore(cwd string, maxCharsPerFile int, dirOverride string) *MemoryStore func (s *MemoryStore) MaxKB() int func (s *MemoryStore) SetWriteObserver(fn MemoryWriteObserver) func (s *MemoryStore) Dir() string @@ -83,21 +128,13 @@ func (s *MemoryStore) Add(file, content, note string) (string, error) func (s *MemoryStore) Replace(file, oldText, newContent, note string) (string, error) func (s *MemoryStore) Remove(file, oldText, note string) (string, error) -type ActionPermissions struct { - AllowCreate bool - AllowUpdate bool - AllowDelete bool - AllowUpdateUserCreated bool -} - -func DefaultActionPermissions() ActionPermissions - type SkillManager struct { /* unexported */ } -func NewSkillManager(cwd string, perms ActionPermissions) *SkillManager -func (m *SkillManager) Perms() ActionPermissions +func NewSkillManager(cwd string, perms SkillPermissions) *SkillManager +func (m *SkillManager) Perms() SkillPermissions func (m *SkillManager) SetWriteObserver(fn SkillWriteObserver) func (m *SkillManager) Inventory() []SkillInfo +func (m *SkillManager) Read(name string) (string, error) func (m *SkillManager) Create(name, description, body, level, note string) (string, error) func (m *SkillManager) Edit(name, body, note string) (string, error) func (m *SkillManager) Patch(name, oldText, newText string, replaceAll bool, note string) (string, error) @@ -108,7 +145,7 @@ func (m *SkillManager) Delete(name, note string) (string, error) ## Internals -- `Reviewer` owns cadence counters and the at-most-one-in-flight gate. It +- `Reviewer` owns the at-most-one-in-flight gate. It observes completed `core.Result` values and launches the injected `ReviewFunc` on a background goroutine. - `RunReview` builds a restricted review agent with only `memory_write` and @@ -139,8 +176,8 @@ func (m *SkillManager) Delete(name, note string) (string, error) ## Tests ``` -internal/selflearn/reviewer_test.go — cadence, single-flight behavior, - seeding, and result filtering. +internal/selflearn/reviewer_test.go — model-request trigger, single-flight, + scoping, and result filtering. internal/selflearn/concurrency_test.go — snapshot copying and race safety. internal/selflearn/fork_test.go — fork prompt/tool restrictions and inherited system state. @@ -151,8 +188,26 @@ internal/selflearn/skill_test.go — skill create/update/delete, internal/selflearn/config_test.go — settings resolution and defaults. internal/selflearn/fixes_test.go — regression coverage for security and patch edge cases. +internal/setting/selflearn_test.go — permission schema migration. +internal/app/selflearn_review_fixes_test.go — runtime gates, layered overrides, + and live-workspace stores. ``` +### Testing the trigger manually + +1. `/evolve` → keep at least one skill permission on (or enable memory), then + **Save**. Saving rebuilds the agent so the `Evolve` tool is injected. +2. Confirm injection: ask the agent to list its tools — `Evolve` appears only + when self-learning is active. +3. Do skill-worthy work, then have the agent call `Evolve` (e.g. reason + "testing the trigger"). +4. Watch: the status-bar indicator (`evolving… → evolved` / `nothing`), the + `/evolve → Learned skills` inventory + RECENT recap, and + `~/.san/debug.log` (`grep selflearn`). + +Requires an active LLM provider (the fork makes a real call) and +`SAN_DISABLE_SELF_LEARN` unset. + ## See Also - Code: `internal/selflearn/` diff --git a/internal/agent/build.go b/internal/agent/build.go index 5e3b1b04..93e17dba 100644 --- a/internal/agent/build.go +++ b/internal/agent/build.go @@ -43,6 +43,11 @@ type BuildParams struct { DisabledTools map[string]bool MCPTools []core.Tool + // ExtraTools are caller-built conditional tool schemas appended to the + // toolset (e.g. the self-learning Evolve trigger, injected only when + // self-learning is active). Nil ⇒ no extra tools. + ExtraTools []core.ToolSchema + // PermissionRules and PermissionReview are the two stages of the // pre-execution permission gate: the rules stage applies the static rules // (permit/reject/prompt); the review stage is the LLM auto-review consulted @@ -90,6 +95,7 @@ func buildAgent(p BuildParams) (core.Agent, *PermissionGate, error) { schemas := (&tool.Set{ Disabled: p.DisabledTools, AgentDirectory: p.AgentDirectory, + ExtraTools: p.ExtraTools, }).Tools() var adaptOpts []tool.AdaptOption if p.AskUser != nil { diff --git a/internal/app/agent.go b/internal/app/agent.go index afd1cbbc..9bf7a884 100644 --- a/internal/app/agent.go +++ b/internal/app/agent.go @@ -120,31 +120,16 @@ func (m *model) buildAgentParams() agent.BuildParams { // hot-swap it mid-session; the closures below Load it per call. m.rebuildAutopilotReviewer() - // Read stream timeout overrides from settings (low priority) then env vars - // (high priority). Priority: SAN_* env var > settings.json > core default. - // Valid time.Duration strings (e.g. "30s", "3m"); empty/invalid leaves the - // core default (zero value => core picks FirstChunkTimeout=5m, IdleTimeout=60s). - var streamFirstChunk, streamIdle time.Duration - if s := m.services.Setting.StreamFirstChunkTimeout(); s != "" { - if d, err := time.ParseDuration(s); err == nil { - streamFirstChunk = d - } - } - if s := m.services.Setting.StreamIdleTimeout(); s != "" { - if d, err := time.ParseDuration(s); err == nil { - streamIdle = d - } - } - if s := setting.Getenv("STREAM_FIRST_CHUNK_TIMEOUT"); s != "" { - if d, err := time.ParseDuration(s); err == nil { - streamFirstChunk = d - } - } - if s := setting.Getenv("STREAM_IDLE_TIMEOUT"); s != "" { - if d, err := time.ParseDuration(s); err == nil { - streamIdle = d - } - } + // Stream timeout overrides. Priority: SAN_* env var > settings.json > core + // default (zero value => core picks FirstChunkTimeout=5m, IdleTimeout=60s). + streamFirstChunk := firstValidDuration( + setting.Getenv("STREAM_FIRST_CHUNK_TIMEOUT"), + m.services.Setting.StreamFirstChunkTimeout(), + ) + streamIdle := firstValidDuration( + setting.Getenv("STREAM_IDLE_TIMEOUT"), + m.services.Setting.StreamIdleTimeout(), + ) return agent.BuildParams{ Provider: m.env.LLMProvider, @@ -164,6 +149,10 @@ func (m *model) buildAgentParams() agent.BuildParams { MCPTools: mcpTools, HookEngine: m.services.Hook, + // Inject the Evolve trigger tool (tailored to the enabled capabilities) + // so the model can queue its own self-learning reviews. + ExtraTools: m.selfLearnExtraTools(), + AskUser: func(ctx context.Context, req *tool.QuestionRequest) (*tool.QuestionResponse, error) { return m.conv.AgentToUI.Ask(ctx, 0, req) }, @@ -352,6 +341,21 @@ func marshalPermInput(args map[string]any) json.RawMessage { return data } +// firstValidDuration returns the first value that parses as a time.Duration +// (e.g. "30s", "3m"), in priority order. Empty/invalid values are skipped; +// zero means "no override" and lets the core default apply. +func firstValidDuration(vals ...string) time.Duration { + for _, v := range vals { + if v == "" { + continue + } + if d, err := time.ParseDuration(v); err == nil { + return d + } + } + return 0 +} + // ============================================================ // Agent lifecycle (delegates to services.Agent) // ============================================================ @@ -363,7 +367,15 @@ func marshalPermInput(args map[string]any) json.RawMessage { // the input twice. Pass "" when the caller hasn't yet appended the message. func (m *model) ensureAgentSession(pendingSend string) (tea.Cmd, error) { if m.services.Agent.Active() { - return nil, nil + // The Evolve toolset is fixed at agent-build time. If the enabled + // capabilities drifted since the build (a /evolve save, an external + // settings edit), stop the stale session and fall through to rebuild + // with the current toolset — the one choke point every turn passes + // through, so all drift sources are covered uniformly. + if m.selfLearnCapabilities() == m.agentEvolveCaps { + return nil, nil + } + m.services.Agent.Stop() } params := m.buildAgentParams() @@ -385,11 +397,13 @@ func (m *model) ensureAgentSession(pendingSend string) (tea.Cmd, error) { return nil, err } + // Record what the toolset was built with, for the drift check above. + m.agentEvolveCaps = m.selfLearnCapabilities() + // Wire L1 self-learning *after* Agent.Start so the ReviewFunc can capture // the live Agent + System for its fork. Builds nothing if both arms are - // off (§3.1 zero-overhead guarantee). pendingSend is forwarded so the - // reviewer's cadence seed skips the in-flight user turn. - m.wireSelfLearn(params, pendingSend) + // off (§3.1 zero-overhead guarantee). + m.wireSelfLearn(params) cmds := []tea.Cmd{ conv.DrainAgentOutbox(m.services.Agent.Outbox()), @@ -449,7 +463,13 @@ func (m *model) wireReminderProviders() { // Kept as its own scope so agent-written entries never mix with the // user-authored memory above. m.services.Reminder.Register(reminder.NewProvider(reminder.ProviderMemoryAuto, func() string { - body, ok := system.LoadAutoMemory(m.env.CWD) + // Honor a configured memory storage path so the injected memory matches + // where the reviewer writes. + override := "" + if snap := m.services.Setting.Snapshot(); snap != nil { + override = snap.SelfLearn.Memory.Path + } + body, ok := system.LoadAutoMemoryAt(system.ResolveAutoMemoryDir(m.env.CWD, override)) if !ok { return "" } diff --git a/internal/app/init.go b/internal/app/init.go index 640bbe4d..8fed500f 100644 --- a/internal/app/init.go +++ b/internal/app/init.go @@ -121,6 +121,7 @@ func discoverPlugins(cwd string) { func (m *model) reloadProjectServices(cwd string) { setting.Initialize(setting.Options{CWD: cwd}) m.services.Setting = setting.Default() + m.learnedStores.Update(cwd, m.services.Setting) skill.Initialize(skill.Options{CWD: cwd, PluginSkillPaths: pluginSkillPaths}) m.services.Skill = skill.Default() diff --git a/internal/app/input/config_appearance.go b/internal/app/input/config_appearance.go index 106198a3..d1395991 100644 --- a/internal/app/input/config_appearance.go +++ b/internal/app/input/config_appearance.go @@ -168,7 +168,7 @@ func (p *appearancePanel) HintLine() string { return keycap("↑↓") + " navigate " + keycap("enter") + " apply" } -func (p *appearancePanel) Render(width int) string { +func (p *appearancePanel) Render(width, _ int) string { var b strings.Builder prevSection := "" diff --git a/internal/app/input/config_appearance_test.go b/internal/app/input/config_appearance_test.go index 4a1a17b3..d697e3ed 100644 --- a/internal/app/input/config_appearance_test.go +++ b/internal/app/input/config_appearance_test.go @@ -119,7 +119,7 @@ func TestAppearancePanelEnterSaveFailureSurfacesError(t *testing.T) { if p.themeBaseline != "auto" { t.Fatalf("themeBaseline should be untouched on failure, got %q", p.themeBaseline) } - if out := p.Render(80); !strings.Contains(out, "couldn't save") { + if out := p.Render(80, 20); !strings.Contains(out, "couldn't save") { t.Fatalf("Render should surface the save error, got:\n%s", out) } } @@ -167,24 +167,6 @@ func TestAppearancePanelContextBarSavesAndEmits(t *testing.T) { } } -// TestConfigSelectorArrowSwitchesPanels confirms ←/→ cycle the registered -// panels (and wrap), the navigation this feature swapped in for ctrl-tab. -func TestConfigSelectorArrowSwitchesPanels(t *testing.T) { - c := NewConfigSelector(nil) - c.Enter(120, 40) - if got := c.ActivePanel().Title(); got != "self-learning" { - t.Fatalf("default panel = %q, want self-learning", got) - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyRight}) - if got := c.ActivePanel().Title(); got != "appearance" { - t.Fatalf("after right = %q, want appearance", got) - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyRight}) // wrap - if got := c.ActivePanel().Title(); got != "self-learning" { - t.Fatalf("after right wrap = %q, want self-learning", got) - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyLeft}) // wrap back - if got := c.ActivePanel().Title(); got != "appearance" { - t.Fatalf("after left wrap = %q, want appearance", got) - } -} +// /config and /evolve each host a single panel today, so the shell's tab +// switching is dormant; it reactivates untested-code-free when a second +// panel is registered (see PanelPopup.HandleKeypress's tab handling). diff --git a/internal/app/input/config_panel.go b/internal/app/input/config_panel.go deleted file mode 100644 index c306c440..00000000 --- a/internal/app/input/config_panel.go +++ /dev/null @@ -1,243 +0,0 @@ -// /config popup shell. ConfigSelector frames one or more sub-panels with -// a "/config" breadcrumb and a tab strip when multiple panels are -// registered, an esc/cancel handler, and centered full-width placement -// that matches the /plugin and /model overlays. Each sub-panel implements -// Panel and owns its own body + hint line. To add a new panel: implement -// Panel and append it to the panels slice in NewConfigSelector. -package input - -import ( - "strings" - - tea "charm.land/bubbletea/v2" - "charm.land/lipgloss/v2" - - "github.com/genai-io/san/internal/app/kit" - "github.com/genai-io/san/internal/setting" -) - -// Panel is one /config sub-panel. -// -// Lifecycle: -// - Enter(): reset working state on (re)activation. -// - HandleKey(): handle one keypress; (cmd, done) — done=true asks the -// shell to dismiss the popup (e.g. after Save). -// - Render(width): the panel body, framed by the shell. -// - HintLine(): the muted bottom hint (e.g. "↑↓ navigate · …"). Shown -// under the body; the shell appends its own "· esc cancel". -type Panel interface { - Title() string - Enter() - HandleKey(msg tea.KeyMsg) (tea.Cmd, bool) - Render(width int) string - HintLine() string - // Dirty reports whether the panel has unsaved edits. The shell uses - // this to pin a "● unsaved" tag to the top-right of the header. - Dirty() bool -} - -// ConfigSavedMsg is emitted on a successful Save so the app can show a -// transient confirmation. SavedSelfLearn carries the snapshot that was -// just written so the consumer can compare it against the post-Reload -// effective state and detect cross-level overrides (settings merger ORs -// Enabled across user+project). -type ConfigSavedMsg struct { - Scope string - SavedSelfLearn setting.SelfLearnSettings -} - -// ConfigSelector is the /config popup. Self-Learning and Appearance are -// registered today; adding a sibling panel (Provider, Permissions, …) is a -// one-liner in NewConfigSelector. -type ConfigSelector struct { - panels []Panel - index int - - active bool - width int - height int -} - -// NewConfigSelector wires the popup with its set of panels. Order is -// preserved by the tab strip. -func NewConfigSelector(settings *setting.Settings) ConfigSelector { - return ConfigSelector{ - panels: []Panel{newSelfLearnPanel(settings), newAppearancePanel(settings)}, - } -} - -// Enter activates the popup with the first panel focused. -func (c *ConfigSelector) Enter(width, height int) { - c.width = width - c.height = height - c.active = true - if c.index >= len(c.panels) { - c.index = 0 - } - if p := c.activePanel(); p != nil { - p.Enter() - } -} - -// IsActive implements the popup interface. -func (c *ConfigSelector) IsActive() bool { return c.active } - -// HandleKeypress implements the popup interface. Esc dismisses the popup; -// ←/→ switch panels (when more than one is registered); everything else -// delegates to the active panel. Panels reserve ↑/↓ for in-panel -// navigation, so left/right stays free for the tab strip. -func (c *ConfigSelector) HandleKeypress(msg tea.KeyMsg) tea.Cmd { - if !c.active { - return nil - } - switch msg.String() { - case "esc": - c.active = false - return nil - case "left", "right": - // Only intercept when there's more than one panel; otherwise fall - // through so the active panel can handle left/right itself. - if len(c.panels) > 1 { - step := 1 - if msg.String() == "left" { - step = -1 - } - c.index = (c.index + step + len(c.panels)) % len(c.panels) - c.panels[c.index].Enter() - return nil - } - } - p := c.activePanel() - if p == nil { - return nil - } - cmd, done := p.HandleKey(msg) - if done { - c.active = false - } - return cmd -} - -// Render frames the active panel with breadcrumb + tab strip on top, the -// panel body, and the combined hint line at the bottom. Centered on screen -// via lipgloss.Place, matching /plugin — but with a capped width so form -// rows don't sprawl across an ultra-wide terminal (values would otherwise -// drift far from their labels). -func (c *ConfigSelector) Render() string { - if !c.active || len(c.panels) == 0 { - return "" - } - p := c.activePanel() - boxWidth, boxHeight := c.boxSize() - gutter := 2 // horizontal gutter on each side of body content - innerWidth := boxWidth - 2*gutter // width body rows render to - pad := strings.Repeat(" ", gutter) // gutter prefix for non-rule lines - - // Top/bottom rules extend the full box width so they read as section - // chrome, not as content indented into the gutter. - rule := configRuleStyle.Render(strings.Repeat("─", boxWidth)) - - // indent indents every line of s by the gutter except blank lines - // (which stay blank so they don't carry trailing whitespace). - indent := func(s string) string { - lines := strings.Split(s, "\n") - for i, ln := range lines { - if ln == "" { - continue - } - lines[i] = pad + ln - } - return strings.Join(lines, "\n") - } - - var b strings.Builder - b.WriteString(indent(c.renderHeader(innerWidth))) - b.WriteString("\n") - b.WriteString(rule) - b.WriteString("\n\n") - b.WriteString(indent(p.Render(innerWidth))) - b.WriteString("\n") - b.WriteString(rule) - b.WriteString("\n") - b.WriteString(indent(c.renderHint(p.HintLine()))) - - box := lipgloss.NewStyle(). - Width(boxWidth). - Height(boxHeight). - Padding(1, 0). // vertical padding only — gutter is in-content - Render(b.String()) - // Center the popup so it sits balanced on a wide terminal instead of - // hugging the left edge. - return lipgloss.Place(c.width, c.height-2, lipgloss.Center, lipgloss.Top, box) -} - -// boxSize sizes the popup the same way /plugin does: width fills the -// terminal minus a 6-col margin, height fills it minus a 4-row margin. -// No upper cap — the popup stretches with the terminal so the form -// reads as a real overlay, not a narrow card. -func (c *ConfigSelector) boxSize() (w, h int) { - w = max(60, c.width-6) - h = max(18, c.height-4) - return w, h -} - -// ActivePanel returns the currently focused panel; nil when none are -// registered. Exported for tests. -func (c *ConfigSelector) ActivePanel() Panel { return c.activePanel() } - -func (c *ConfigSelector) activePanel() Panel { - if len(c.panels) == 0 { - return nil - } - return c.panels[c.index] -} - -// renderHeader returns the breadcrumb (or tab pills, when multiple -// panels are registered) with an optional "● unsaved" tag pinned to -// the right edge of the header line. -func (c *ConfigSelector) renderHeader(width int) string { - var left string - if len(c.panels) == 1 { - left = configBreadcrumbDimStyle.Render("/config") + - configBreadcrumbDimStyle.Render(" › ") + - configBreadcrumbStyle.Render(c.panels[0].Title()) - } else { - tabs := make([]kit.PanelTab, len(c.panels)) - for i, p := range c.panels { - tabs[i] = kit.PanelTab{Name: p.Title(), Show: true} - } - return configBreadcrumbDimStyle.Render("/config") + "\n\n" + - kit.RenderPanelTabs(tabs, c.index) - } - - right := "" - if c.activePanel() != nil && c.activePanel().Dirty() { - right = configUnsavedDotStyle.Render("●") + " " + - configUnsavedTextStyle.Render("unsaved") - } - if right == "" { - return left - } - gap := max(width-lipgloss.Width(left)-lipgloss.Width(right), 1) - return left + strings.Repeat(" ", gap) + right -} - -func (c *ConfigSelector) renderHint(panelHint string) string { - parts := []string{} - if panelHint != "" { - parts = append(parts, panelHint) - } - parts = append(parts, "esc cancel") - if len(c.panels) > 1 { - parts = append(parts, "←→ switch panel") - } - return kit.HintLine(parts...) -} - -var ( - configBreadcrumbDimStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) - configBreadcrumbStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Accent).Bold(true) - configRuleStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) - configUnsavedDotStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Warning).Bold(true) - configUnsavedTextStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Warning) -) diff --git a/internal/app/input/config_selflearn.go b/internal/app/input/config_selflearn.go deleted file mode 100644 index da9bfa70..00000000 --- a/internal/app/input/config_selflearn.go +++ /dev/null @@ -1,571 +0,0 @@ -// /config Self-Learning panel: edits setting.SelfLearnSettings against a -// working snapshot, validates the §3.1 invariants inline, and writes to -// either the user-level or project-level settings file on Save. -package input - -import ( - "fmt" - "strconv" - "strings" - "unicode" - - tea "charm.land/bubbletea/v2" - "charm.land/lipgloss/v2" - - "github.com/genai-io/san/internal/app/kit" - "github.com/genai-io/san/internal/setting" -) - -type selfLearnPanel struct { - settings *setting.Settings - - // snap is the working buffer; Save merges it back to disk. - snap setting.SelfLearnSettings - // baseline captures snap as it was at Enter() time so the panel can - // flag unsaved edits in the top-right corner. - baseline setting.SelfLearnSettings - scope string // "user" | "project" - - cursor int - editing bool - editingBuffer string -} - -func newSelfLearnPanel(settings *setting.Settings) *selfLearnPanel { - return &selfLearnPanel{settings: settings, scope: "user"} -} - -func (p *selfLearnPanel) Title() string { return "self-learning" } - -func (p *selfLearnPanel) Enter() { - p.editing = false - p.editingBuffer = "" - if p.settings == nil { - p.snap = setting.SelfLearnSettings{} - } else if data := p.settings.Snapshot(); data != nil { - p.snap = data.SelfLearn - } - p.baseline = p.snap - p.cursor = findEditable(p.rows(), 0, +1, 0) -} - -// Dirty reports whether the working snapshot diverges from the disk -// baseline — the shell uses it to pin the "● unsaved" indicator. -func (p *selfLearnPanel) Dirty() bool { return p.snap != p.baseline } - -func (p *selfLearnPanel) HandleKey(msg tea.KeyMsg) (tea.Cmd, bool) { - if p.editing { - return p.handleEditingKey(msg), false - } - rows := p.rows() - if p.cursor >= len(rows) { - p.cursor = findEditable(rows, 0, +1, 0) - } - switch msg.String() { - case "up", "k": - p.cursor = findEditable(rows, p.cursor-1, -1, p.cursor) - case "down", "j": - p.cursor = findEditable(rows, p.cursor+1, +1, p.cursor) - case "tab": - if p.scope == "user" { - p.scope = "project" - } else { - p.scope = "user" - } - case "space": - if r := rows[p.cursor]; r.kind == rowBool && r.toggle != nil { - r.toggle(&p.snap) - } - case "enter": - row := rows[p.cursor] - switch row.kind { - case rowBool: - if row.toggle != nil { - row.toggle(&p.snap) - } - case rowInt: - p.editing = true - p.editingBuffer = strconv.Itoa(row.intGetter(&p.snap)) - case rowSave: - if err := p.snap.Validate(); err != nil { - return nil, false - } - userLevel := p.scope == "user" - if err := setting.UpdateSelfLearnAt(p.snap, userLevel); err != nil { - return nil, false - } - scope := p.scope - saved := p.snap - return func() tea.Msg { - return ConfigSavedMsg{Scope: scope, SavedSelfLearn: saved} - }, true - } - } - return nil, false -} - -func (p *selfLearnPanel) handleEditingKey(msg tea.KeyMsg) tea.Cmd { - switch msg.String() { - case "esc": - p.editing = false - p.editingBuffer = "" - case "enter": - row := p.rows()[p.cursor] - if v, err := strconv.Atoi(p.editingBuffer); err == nil { - if v < row.intMin { - v = row.intMin - } - if v > row.intMax { - v = row.intMax - } - row.intSetter(&p.snap, v) - } - p.editing = false - p.editingBuffer = "" - case "backspace": - if n := len(p.editingBuffer); n > 0 { - p.editingBuffer = p.editingBuffer[:n-1] - } - default: - if t := msg.Key().Text; len(t) == 1 && t[0] >= '0' && t[0] <= '9' && len(p.editingBuffer) < 4 { - p.editingBuffer += t - } - } - return nil -} - -// HintLine renders the bottom hint as monospace keycaps. -func (p *selfLearnPanel) HintLine() string { - return keycap("↑↓") + " navigate " + - keycap("space") + " toggle " + - keycap("enter") + " edit/save " + - keycap("tab") + " scope" -} - -// Render draws the panel body. -func (p *selfLearnPanel) Render(width int) string { - rows := p.rows() - validationErr := p.snap.Validate() - - var b strings.Builder - b.WriteString(p.renderScopeControl()) - b.WriteString("\n\n") - - rail := "" // current section's styled rail prefix - sectionEnabled := true // whether the current section is "on" - for i, row := range rows { - switch row.kind { - case rowSectionHeader: - b.WriteString(p.renderSectionHeader(row, width)) - rail = p.railFor(row) - sectionEnabled = row.enabledFn == nil || row.enabledFn(&p.snap) - case rowSubHeader: - indentPad := strings.Repeat(" ", contentCol(row.indent)-1) - line := indentPad + selflearnSubHeaderStyle.Render(row.label) - b.WriteString(rail + p.maybeDim(sectionEnabled, line)) - case rowSpacer: - // Skip the rail on trailing spacers so the bar visibly closes - // at the last content row of the section. - if !p.isTrailingSpacer(rows, i) { - b.WriteString(rail) - } - case rowSave: - b.WriteString(p.renderSaveRow(i, validationErr)) - case rowBool: - line := p.renderBoolRow(i, row, width) - b.WriteString(p.withRail(rail, p.maybeDim(sectionEnabled, line))) - case rowInt: - line := p.renderIntRow(i, row, width) - b.WriteString(p.withRail(rail, p.maybeDim(sectionEnabled, line))) - } - b.WriteString("\n") - } - - if validationErr != nil { - b.WriteString("\n") - b.WriteString(selflearnErrorStyle.Render("⚠ " + validationErr.Error())) - b.WriteString("\n") - } - return b.String() -} - -// maybeDim wraps the rendered row in Faint when the parent section is -// off, so disabled-section children stop reading as "active". -func (p *selfLearnPanel) maybeDim(enabled bool, line string) string { - if enabled { - return line - } - return selflearnFaintStyle.Render(line) -} - -// isTrailingSpacer reports whether row i is a spacer with no further -// content rows in the current section. -func (p *selfLearnPanel) isTrailingSpacer(rows []configRow, i int) bool { - for j := i + 1; j < len(rows); j++ { - switch rows[j].kind { - case rowSpacer: - continue - case rowSectionHeader, rowSave: - return true - default: - return false - } - } - return true -} - -// renderScopeControl is a two-segment selector with the active segment -// as a filled pill. The "scope" label is dropped — the two visible -// segments (user / project) are self-explanatory. -func (p *selfLearnPanel) renderScopeControl() string { - seg := func(name string) string { - if p.scope == name { - return selflearnScopeActiveStyle.Render(name) - } - return selflearnScopeIdleStyle.Render(name) - } - sep := selflearnMutedStyle.Render(" · ") - return seg("user") + sep + seg("project") -} - -// renderSectionHeader prints "MEMORY ─────…" — label in caps, then a -// hairline rule filling the rest of the row. -func (p *selfLearnPanel) renderSectionHeader(row configRow, width int) string { - label := strings.ToUpper(row.label) - ruleLen := max(width-len(label)-1, 1) - return selflearnSectionStyle.Render(label) + " " + - selflearnRuleStyle.Render(strings.Repeat("─", ruleLen)) -} - -// railFor returns the left-edge rail for a section's content. The shape -// AND color carry the on/off signal so the two states pop visually: -// - on: "┃" — heavy vertical bar in accent green -// - off: "╎" — dashed light bar in muted text-dim -// -// This way the active section's rail clearly outweighs the section- -// divider hairlines ("MEMORY ────"), making "which one is running" the -// loudest signal on the panel. -func (p *selfLearnPanel) railFor(row configRow) string { - if row.enabledFn != nil && row.enabledFn(&p.snap) { - return selflearnRailOnStyle.Render("┃") - } - return selflearnRailOffStyle.Render("╎") -} - -// withRail prepends the section rail to a row, consuming its first -// blank column so the rest of the column math stays consistent. -func (p *selfLearnPanel) withRail(rail, row string) string { - if rail == "" { - return row - } - if len(row) > 0 && row[0] == ' ' { - return rail + row[1:] - } - return rail + row -} - -// keycap renders a key label as a bg-filled pill so it doesn't read as -// a checkbox. The fill is the kit's neutral search-input gray so the -// keycap feels like a physical key cap, not a [ ] toggle. -func keycap(s string) string { - return selflearnKeycapStyle.Render(" " + capitalizeKeyLabel(s) + " ") -} - -// capitalizeKeyLabel title-cases each part of a key combo so hints read as -// proper key names: "ctrl+r" → "Ctrl+R", "enter" → "Enter". Arrow glyphs -// (↑↓, ←→) and other non-letters pass through unchanged. -func capitalizeKeyLabel(s string) string { - parts := strings.Split(s, "+") - for i, p := range parts { - if p == "" { - continue - } - r := []rune(p) - r[0] = unicode.ToUpper(r[0]) - parts[i] = string(r) - } - return strings.Join(parts, "+") -} - -// ── Row rendering ─────────────────────────────────────────────────────── - -// Layout columns. Every row's "content" (bracket for bool, value for int) -// starts at a column derived from its indent (indentStep cols per level); -// the cursor caret sits cursorWidth cols before that. Section headers go -// at indent 0 (col 0). Rows directly under a section: indent 1 (col 4). -// Sub-sections under those: header at indent 1, rows at indent 2 (col 8). -const ( - indentStep = 4 - cursorWidth = 2 -) - -// contentCol returns the column where the row's leftmost rendered content -// (bracket / value / save button) starts, for the given indent. -func contentCol(indent int) int { return indent * indentStep } - -func (p *selfLearnPanel) cursorMark(i int) string { - if i == p.cursor { - return selflearnCursorStyle.Render("▸ ") - } - return strings.Repeat(" ", cursorWidth) -} - -// cursorPad returns the leading whitespace + cursor caret so the next -// glyph lands exactly at contentCol(indent). -func (p *selfLearnPanel) cursorPad(i, indent int) string { - at := max(contentCol(indent)-cursorWidth, 0) - return strings.Repeat(" ", at) + p.cursorMark(i) -} - -func (p *selfLearnPanel) renderBoolRow(i int, row configRow, _ int) string { - mark := "[ ]" - if row.boolGetter(&p.snap) { - mark = selflearnCheckStyle.Render("[✓]") - } - line := p.cursorPad(i, row.indent) + mark + " " + row.label - if row.advHint != "" { - line += " " + selflearnHintStyle.Render(row.advHint) - } - return line -} - -func (p *selfLearnPanel) renderIntRow(i int, row configRow, _ int) string { - value := strconv.Itoa(row.intGetter(&p.snap)) - if p.editing && i == p.cursor { - value = p.editingBuffer + "_" - } - // Label-first phrase: "Run every (10) user turns" reads as a sentence. - // The label sits one bracket-width past where a bool row's "[" goes, - // so labels align with bool-row labels at the same indent. - labelStart := contentCol(row.indent) + 4 // 4 = bracket width "[ ] " - leftPad := strings.Repeat(" ", labelStart-cursorWidth) - line := leftPad + p.cursorMark(i) + row.label + " " + valueChip(value) - if row.unit != "" { - line += " " + selflearnMutedStyle.Render(row.unit) - } - if row.footnote != nil { - fn := row.footnote(row.intGetter(&p.snap)) - line += selflearnMutedStyle.Render(" ~ " + fn) - } - return line -} - -// valueChip wraps a numeric value in chip-style brackets so it reads as -// an editable input: muted parens around an accent-bold value. -func valueChip(value string) string { - return selflearnChipBracketStyle.Render("(") + - selflearnValueStyle.Render(value) + - selflearnChipBracketStyle.Render(")") -} - -func (p *selfLearnPanel) renderSaveRow(i int, validationErr error) string { - style := selflearnSaveButtonStyle - if validationErr != nil { - style = selflearnSaveButtonDisabledStyle - } - btn := style.Render("Save") - tail := selflearnMutedStyle.Render(" or " + keycap("esc") + selflearnMutedStyle.Render(" to discard")) - return p.cursorPad(i, 1) + btn + tail -} - -// ── Row kinds and layout ──────────────────────────────────────────────── - -// rowKind discriminates the rendered row types: bool toggle, int with a -// clamped range, the save action, two header levels, a blank spacer, and -// the advanced-action hint. -type rowKind int - -const ( - rowBool rowKind = iota - rowInt - rowSave - rowSectionHeader // big section title (Memory / Skills) - rowSubHeader // sub-section title (Allowed actions / Advanced) - rowSpacer // blank line -) - -// configRow is one renderable row. Fields unused by the row's kind stay zero. -type configRow struct { - kind rowKind - label string - toggle func(*setting.SelfLearnSettings) - boolGetter func(*setting.SelfLearnSettings) bool - intGetter func(*setting.SelfLearnSettings) int - intSetter func(*setting.SelfLearnSettings, int) - // enabledFn is set on rowSectionHeader to tell the renderer whether - // the section is "on" (drives the vertical rail color). - enabledFn func(*setting.SelfLearnSettings) bool - intMin int - intMax int - unit string // for rowInt — muted suffix after the value (e.g. "user turns", "KB") - footnote func(int) string // for rowInt — optional muted inline footnote after the label - advHint string // for rowBool — optional inline "⚠ …" hint after the label - indent int -} - -// editable reports whether the row can hold the cursor. Derived from kind: -// the three actionable kinds (toggle, edit-in-place, save) are editable; -// section headers, sub-headers, and spacers are not. -func (r configRow) editable() bool { - return r.kind == rowBool || r.kind == rowInt || r.kind == rowSave -} - -func (p *selfLearnPanel) rows() []configRow { - return []configRow{ - {kind: rowSectionHeader, label: "Memory", enabledFn: func(s *setting.SelfLearnSettings) bool { return s.Memory.Enabled }}, - { - kind: rowBool, - label: "Enable memory-evolving", - indent: 1, - boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Memory.Enabled }, - toggle: func(s *setting.SelfLearnSettings) { s.Memory.Enabled = !s.Memory.Enabled }, - }, - { - kind: rowInt, - label: "Run every", - unit: "user turns", - indent: 1, - intGetter: func(s *setting.SelfLearnSettings) int { return s.Memory.ResolvedEveryTurns() }, - intSetter: func(s *setting.SelfLearnSettings, v int) { s.Memory.EveryTurns = v }, - intMin: 1, - intMax: 100, - }, - { - kind: rowInt, - label: "Max size", - unit: "KB", - indent: 1, - intGetter: func(s *setting.SelfLearnSettings) int { return s.Memory.ResolvedMaxKB() }, - intSetter: func(s *setting.SelfLearnSettings, v int) { s.Memory.MaxKB = v }, - intMin: 1, - intMax: setting.SelfLearnMaxMemoryKB, - footnote: func(v int) string { - return fmt.Sprintf("%d EN words / %d 中文字 (UTF-8)", v*180, v*340) - }, - }, - {kind: rowSpacer}, - {kind: rowSectionHeader, label: "Skills", enabledFn: func(s *setting.SelfLearnSettings) bool { return s.Skills.Enabled }}, - { - kind: rowBool, - label: "Enable skill-evolving", - indent: 1, - boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Skills.Enabled }, - toggle: func(s *setting.SelfLearnSettings) { s.Skills.Enabled = !s.Skills.Enabled }, - }, - { - kind: rowInt, - label: "Run every", - unit: "tool iterations", - indent: 1, - intGetter: func(s *setting.SelfLearnSettings) int { return s.Skills.ResolvedEveryToolIters() }, - intSetter: func(s *setting.SelfLearnSettings, v int) { s.Skills.EveryToolIters = v }, - intMin: 1, - intMax: 100, - }, - {kind: rowSpacer}, - {kind: rowSubHeader, label: "Allowed actions (agent-created scope)", indent: 1}, - { - kind: rowBool, - label: "Create new skills", - indent: 2, - boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Skills.AllowCreate() }, - toggle: func(s *setting.SelfLearnSettings) { s.Skills.DenyCreate = !s.Skills.DenyCreate }, - }, - { - kind: rowBool, - label: "Update existing skills", - indent: 2, - boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Skills.AllowUpdate() }, - toggle: func(s *setting.SelfLearnSettings) { s.Skills.DenyUpdate = !s.Skills.DenyUpdate }, - }, - { - kind: rowBool, - label: "Delete obsolete skills", - indent: 2, - boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Skills.AllowDelete() }, - toggle: func(s *setting.SelfLearnSettings) { s.Skills.DenyDelete = !s.Skills.DenyDelete }, - }, - {kind: rowSpacer}, - {kind: rowSubHeader, label: "Advanced", indent: 1}, - { - kind: rowBool, - label: "Update user-authored skills", - indent: 2, - boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Skills.AllowUpdateUserCreated }, - toggle: func(s *setting.SelfLearnSettings) { s.Skills.AllowUpdateUserCreated = !s.Skills.AllowUpdateUserCreated }, - advHint: "⚠ rewrites your authored skill files", - }, - {kind: rowSpacer}, - {kind: rowSave, label: "Save"}, - } -} - -// findEditable walks rows from start in direction step (+1 forward, -// −1 backward) until it hits an editable row. Returns fallback when -// no editable row is found in that direction — start ≥ len(rows) for -// the first-row lookup, the cursor itself for next/prev so navigation -// past the last actionable row stays put. -func findEditable(rows []configRow, start, step, fallback int) int { - for i := start; i >= 0 && i < len(rows); i += step { - if rows[i].editable() { - return i - } - } - return fallback -} - -// ── Styles ────────────────────────────────────────────────────────────── - -var ( - selflearnSectionStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Accent).Bold(true) - selflearnSubHeaderStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text).Bold(true) - selflearnMutedStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted) - selflearnHintStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Warning).Italic(true) - selflearnErrorStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Error) - selflearnCursorStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Accent).Bold(true) - selflearnCheckStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Success) - selflearnValueStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Accent).Underline(true) - selflearnRuleStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim).Faint(true) - - // Vertical rail along the left of a section's content. Color reflects - // the section's enabled state — green when "on", muted when "off". - selflearnRailOnStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Success).Bold(true) - selflearnRailOffStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) - - // Two-segment scope control: active is a filled pill (accent bg + - // background fg), inactive is a flat padded label so the two segments - // read as a real segmented control. - selflearnScopeActiveStyle = lipgloss.NewStyle(). - Background(kit.CurrentTheme.Accent). - Foreground(kit.CurrentTheme.Background). - Bold(true). - Padding(0, 1) - selflearnScopeIdleStyle = lipgloss.NewStyle(). - Foreground(kit.CurrentTheme.TextDim). - Padding(0, 1) - - // Keycap pill — neutral gray bg + bold text so it visibly diverges - // from "[ ]" checkboxes and reads as a physical key. - selflearnKeycapStyle = lipgloss.NewStyle(). - Background(kit.SearchBg). - Foreground(kit.CurrentTheme.Text). - Bold(true) - - // Faint wrapper for disabled-section children. - selflearnFaintStyle = lipgloss.NewStyle().Faint(true) - - // Save button — filled accent pill when ready, muted pill when the - // snapshot fails validation. - selflearnSaveButtonStyle = lipgloss.NewStyle(). - Background(kit.CurrentTheme.Success). - Foreground(kit.CurrentTheme.Background). - Bold(true). - Padding(0, 2) - selflearnSaveButtonDisabledStyle = lipgloss.NewStyle(). - Background(kit.CurrentTheme.TextDim). - Foreground(kit.CurrentTheme.Background). - Padding(0, 2) - - // Chip-style brackets for editable values: "(10)" with muted parens. - selflearnChipBracketStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) -) diff --git a/internal/app/input/config_selflearn_test.go b/internal/app/input/config_selflearn_test.go deleted file mode 100644 index b34325ac..00000000 --- a/internal/app/input/config_selflearn_test.go +++ /dev/null @@ -1,143 +0,0 @@ -package input - -import ( - "strings" - "testing" - - tea "charm.land/bubbletea/v2" - - "github.com/genai-io/san/internal/setting" -) - -// newTestPopup builds an isolated ConfigSelector + active panel for tests. -// settings=nil so Enter() seeds the snapshot to the zero "feature off" -// baseline without touching disk. -func newTestPopup() (*ConfigSelector, *selfLearnPanel) { - c := NewConfigSelector(nil) - c.Enter(120, 40) - return &c, c.ActivePanel().(*selfLearnPanel) -} - -// TestSelfLearnPanelCursorSkipsHeaders guards against nil-toggle panics: -// the cursor must never land on a section/sub-header / spacer / hint row -// (toggle is nil → Space would crash). -func TestSelfLearnPanelCursorSkipsHeaders(t *testing.T) { - c, p := newTestPopup() - rows := p.rows() - - if !rows[p.cursor].editable() { - t.Fatalf("initial cursor on non-editable row %d (%v)", p.cursor, rows[p.cursor].kind) - } - - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeySpace}) // toggle the first editable row - - for range len(rows) * 2 { - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) - if !rows[p.cursor].editable() { - t.Fatalf("down landed on non-editable row %d (%v)", p.cursor, rows[p.cursor].kind) - } - } - for range len(rows) * 2 { - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyUp}) - if !rows[p.cursor].editable() { - t.Fatalf("up landed on non-editable row %d (%v)", p.cursor, rows[p.cursor].kind) - } - } -} - -// TestConfigSelectorActivatesAndDismisses confirms the popup flips on with -// Enter() and off when Esc is delivered through HandleKeypress. -func TestConfigSelectorActivatesAndDismisses(t *testing.T) { - c, _ := newTestPopup() - if !c.IsActive() { - t.Fatal("Enter should activate the popup") - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEscape}) - if c.IsActive() { - t.Fatal("Esc should deactivate") - } -} - -// TestSelfLearnPanelTogglesBool walks the bool-row flow on Space and Enter. -func TestSelfLearnPanelTogglesBool(t *testing.T) { - c, p := newTestPopup() - // Cursor lands on the first editable row, "Enable memory-evolving". - if p.snap.Memory.Enabled { - t.Fatal("baseline: Memory.Enabled should be false") - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeySpace}) - if !p.snap.Memory.Enabled { - t.Fatal("space should toggle Memory.Enabled true") - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) - if p.snap.Memory.Enabled { - t.Fatal("enter should toggle Memory.Enabled false") - } -} - -// TestSelfLearnPanelIntEditAndClamp drives the int-edit flow: Enter starts -// editing, digits build the buffer, the value clamps to the row's [min,max] -// on commit. -func TestSelfLearnPanelIntEditAndClamp(t *testing.T) { - c, p := newTestPopup() - // Cursor starts on Enable memory-evolving. Down twice to "Max size". - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) // start edit - if !p.editing { - t.Fatal("Enter on int row should start editing") - } - for range 4 { - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyBackspace}) - } - c.HandleKeypress(tea.KeyPressMsg{Code: '9', Text: "9"}) - c.HandleKeypress(tea.KeyPressMsg{Code: '9', Text: "9"}) - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) // commit - if p.editing { - t.Fatal("Enter should commit and exit edit mode") - } - if got := p.snap.Memory.MaxKB; got != setting.SelfLearnMaxMemoryKB { - t.Fatalf("MaxKB clamped: got %d, want %d", got, setting.SelfLearnMaxMemoryKB) - } -} - -// TestConfigSelectorRenderShowsValidationError confirms a §3.1 invalid -// combination surfaces the inline error. -func TestConfigSelectorRenderShowsValidationError(t *testing.T) { - c, p := newTestPopup() - p.snap.Skills.DenyUpdate = true - out := c.Render() - if !strings.Contains(out, `"Create new skills" needs "Update existing skills"`) { - t.Fatalf("Render should surface the §3.1 error, got:\n%s", out) - } -} - -// TestConfigSelectorRenderShowsTabs confirms the "/config" header plus a tab -// strip naming every registered panel makes it into the popup output. (With a -// single panel the header is a breadcrumb instead; today two panels — -// self-learning and appearance — are registered, so it renders tabs.) -func TestConfigSelectorRenderShowsTabs(t *testing.T) { - c, _ := newTestPopup() - out := c.Render() - for _, want := range []string{"/config", "self-learning", "appearance"} { - if !strings.Contains(out, want) { - t.Fatalf("header missing %q from render:\n%s", want, out) - } - } -} - -// TestSelfLearnPanelTabFlipsScope toggles between user / project save targets. -func TestSelfLearnPanelTabFlipsScope(t *testing.T) { - c, p := newTestPopup() - if p.scope != "user" { - t.Fatalf("default scope: got %q, want user", p.scope) - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyTab}) - if p.scope != "project" { - t.Fatalf("after tab: got %q", p.scope) - } - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyTab}) - if p.scope != "user" { - t.Fatalf("after second tab: got %q", p.scope) - } -} diff --git a/internal/app/input/evolve_skills.go b/internal/app/input/evolve_skills.go new file mode 100644 index 00000000..72504b83 --- /dev/null +++ b/internal/app/input/evolve_skills.go @@ -0,0 +1,494 @@ +// The /evolve panel: configures both self-learning arms (skill permissions + +// memory) and manages what the reviewer has learned. +// +// The panel is a small mode machine layered over the config form: +// +// modeForm the config zone (selfLearnForm) — edit + save the settings. +// modeStrategy the Strategy editor — a full-text learning-strategy override. +// modeList the LEARNED inventory — agent-created skills and agent-written +// memory files; esc returns to the form. +// modeView a full, scrollable read-only SKILL.md / memory preview. +// modeConfirm a "delete ?" y/n prompt. +// +// Every mode except modeForm is modal (see modalPanel): while in one the panel +// captures the shell's esc / Tab so those keys drive the sub-view rather than +// dismissing the popup or switching tabs. +// +// Config edits go through the form's user/project Save; inventory deletes are +// immediate, human-initiated actions (not the reviewer's gated writes) and +// take effect on disk at once. +package input + +import ( + "fmt" + "strconv" + + tea "charm.land/bubbletea/v2" + + "github.com/genai-io/san/internal/selflearn" + "github.com/genai-io/san/internal/setting" +) + +// EvolveDeps bundles everything the /evolve panel needs. All fields are +// optional; a zero value builds a usable (inert-inventory, zero-settings) +// popup for tests. +type EvolveDeps struct { + // Workspace returns the live cwd + settings service. The form and the + // learned stores read through it so a project reload is picked up. + Workspace func() (string, *setting.Settings) + Learned LearnedSkillStore + Memory LearnedMemoryStore + // Recent returns the rolling self-learning activity log for the RECENT + // zone of the Learned drill-in. + Recent func() []LearnEvent +} + +// NewEvolveSelector builds the /evolve popup: one self-learning panel covering +// both arms (skills + memory). +func NewEvolveSelector(d EvolveDeps) PanelPopup { + return newPanelPopup("✦", "Self-Learning", "learns as you work", + newEvolvePanel(d)) +} + +// LearnedSkillStore is the app-injected accessor bundle backing the inventory: +// List the agent-created skills, Read one's full SKILL.md for preview, Delete +// one from disk. Any func may be nil (e.g. in tests), in which case that +// capability is inert. +type LearnedSkillStore struct { + List func() []selflearn.SkillInfo + Read func(name string) (string, error) + Delete func(name string) error +} + +// LearnedMemory is a one-line summary of an agent-written memory file, surfaced +// alongside skills in the LEARNED drill-in. +type LearnedMemory struct { + Topic string // display name ("MEMORY.md" for the index, else the topic) + File string // the .md filename backing Read / Delete + Summary string // e.g. "12 lines" +} + +// LearnedMemoryStore is the app-injected accessor bundle backing the memory half +// of the inventory: List the agent-written memory files, Read one, Delete one. +// Any func may be nil, in which case that capability is inert. +type LearnedMemoryStore struct { + List func() []LearnedMemory + Read func(file string) (string, error) + Delete func(file string) error +} + +// learnedItem is one row of the LEARNED drill-in — an agent-created skill or an +// agent-written memory file. kind selects which store backs view / delete. +type learnedItem struct { + kind string // "skill" | "memory" + name string // skill name / memory topic + subtitle string // skill scope / memory summary + desc string // skill description / "" for memory + storeKey string // Read+Delete key: skill name / memory filename +} + +// LearnEvent is one row of the RECENT self-learning activity recap: a write the +// reviewer made, humanized for display. Sourced from the app's rolling activity +// log; Kind is "memory" or "skill", naming the arm that wrote it. +type LearnEvent struct { + Kind string // "memory" | "skill" + Verb string // "created" | "patched" | "removed" | … + Target string // skill name / memory topic + Note string // one-line "what changed" + Ago string // humanized age, e.g. "2m ago" +} + +type skillMode int + +const ( + modeForm skillMode = iota + modeList + modeView + modeConfirm + modeStrategy +) + +type evolvePanel struct { + form selfLearnForm + store LearnedSkillStore + memStore LearnedMemoryStore + recent func() []LearnEvent // nil = no RECENT zone + + strategy strategyEditor + + mode skillMode + items []learnedItem // skills + memory, loaded at Enter / drill-in + // rows is the cached config-row list; rebuilt only when items changes + // (its one input), so per-keypress/per-frame rowsFn calls don't rebuild + // ~20 closures each time. + rows []configRow + events []LearnEvent // RECENT snapshot (both arms), loaded at Enter + invCursor int + actionErr error // last delete/preview failure, surfaced inline; cleared on nav + + // view (modeView) sub-state. + viewName string + viewKind string // "skill" | "memory" — selects the preview title + viewLines []string + viewOffset int +} + +func newEvolvePanel(d EvolveDeps) *evolvePanel { + p := &evolvePanel{ + store: d.Learned, + memStore: d.Memory, + recent: d.Recent, + strategy: newStrategyEditor(selflearn.DefaultStrategy()), + rows: evolveRows(0), + } + p.form = selfLearnForm{ + workspace: d.Workspace, + scope: "user", + rowsFn: func() []configRow { return p.rows }, + } + return p +} + +func (p *evolvePanel) Title() string { return "self-learning" } +func (p *evolvePanel) Dirty() bool { return p.form.Dirty() } +func (p *evolvePanel) HintLine() string { + switch p.mode { + case modeStrategy: + return keycap("enter") + " save " + keycap("esc") + " discard " + keycap("alt+enter") + " newline" + case modeView: + return keycap("↑↓") + " scroll " + keycap("esc") + " back" + case modeConfirm: + return keycap("y") + " delete " + keycap("n") + " cancel" + case modeList: + return keycap("↑↓") + " navigate " + keycap("enter") + " view " + + keycap("d") + " delete " + keycap("esc") + " back" + default: + return p.form.HintLine() + } +} + +// Modal reports the sub-views that capture esc / Tab from the shell — every +// mode except the base config form. +func (p *evolvePanel) Modal() bool { return p.mode != modeForm } + +// Breadcrumb names the open sub-view for the header lockup. +func (p *evolvePanel) Breadcrumb() string { + switch p.mode { + case modeStrategy: + return "strategy" + case modeList, modeConfirm: + return "learned" + case modeView: + return "preview" + default: + return "" + } +} + +func (p *evolvePanel) Enter() { + p.form.Enter() + p.mode = modeForm + p.invCursor = 0 + p.viewOffset = 0 + p.actionErr = nil + p.reloadItems() + p.reloadRecent() +} + +// reloadItems re-reads the on-disk inventory — agent-created skills first, then +// agent-written memory files — and keeps the cursor in range. +func (p *evolvePanel) reloadItems() { + p.items = nil + if p.store.List != nil { + for _, s := range p.store.List() { + p.items = append(p.items, learnedItem{ + kind: "skill", name: s.Name, subtitle: s.Level, desc: s.Description, storeKey: s.Name, + }) + } + } + if p.memStore.List != nil { + for _, m := range p.memStore.List() { + p.items = append(p.items, learnedItem{ + kind: "memory", name: m.Topic, subtitle: m.Summary, storeKey: m.File, + }) + } + } + if p.invCursor >= len(p.items) { + p.invCursor = max(0, len(p.items)-1) + } + // items is the row list's only input, so this is the one rebuild point. + p.rows = evolveRows(len(p.items)) +} + +// reloadRecent snapshots the recent activity log (both memory and skill events — +// one panel now covers both arms). +func (p *evolvePanel) reloadRecent() { + if p.recent == nil { + p.events = nil + return + } + p.events = p.recent() +} + +func (p *evolvePanel) HandleKey(msg tea.KeyMsg) (tea.Cmd, bool) { + switch p.mode { + case modeStrategy: + return p.handleStrategyKey(msg), false + case modeView: + return p.handleViewKey(msg), false + case modeConfirm: + return p.handleConfirmKey(msg), false + case modeList: + return p.handleListKey(msg), false + default: + return p.handleFormKey(msg) + } +} + +// handleFormKey delegates to the config form, then opens whichever entry row +// the user activated: the Strategy editor or the Learned inventory. +func (p *evolvePanel) handleFormKey(msg tea.KeyMsg) (tea.Cmd, bool) { + cmd, done, activated := p.form.HandleKey(msg) + switch activated { + case "strategy": + p.openStrategy() + case "learned": + p.openLearned() + } + return cmd, done +} + +// openLearned drills into the learned-skills inventory sub-view. +func (p *evolvePanel) openLearned() { + p.reloadItems() + p.reloadRecent() + p.invCursor = 0 + p.actionErr = nil + p.mode = modeList +} + +// openStrategy seeds the Strategy editor from the working snapshot and +// enters the modal sub-view. +func (p *evolvePanel) openStrategy() { + p.strategy.open(p.form.snap.Strategy) + p.mode = modeStrategy +} + +// handleStrategyKey drives the Strategy editor: Enter saves the edit back to +// the snapshot and returns; Alt/Shift+Enter inserts a newline; Esc discards +// and returns; everything else feeds the textarea. +func (p *evolvePanel) handleStrategyKey(msg tea.KeyMsg) tea.Cmd { + switch msg.String() { + case "esc": + p.mode = modeForm // discard — leave the working config unchanged + return nil + case "enter": + p.form.snap.Strategy = p.strategy.value() + p.mode = modeForm + return nil + case "alt+enter", "shift+enter": + p.strategy.insertNewline() + return nil + default: + return p.strategy.handleKey(msg) + } +} + +// HandlePaste feeds bracketed paste into the Strategy editor. +func (p *evolvePanel) HandlePaste(content string) { + if p.mode == modeStrategy { + p.strategy.insert(content) + } +} + +func (p *evolvePanel) handleListKey(msg tea.KeyMsg) tea.Cmd { + p.actionErr = nil + switch msg.String() { + case "esc": + p.mode = modeForm // back out of the inventory to the config + case "up", "k": + if p.invCursor > 0 { + p.invCursor-- + } + case "down", "j": + if p.invCursor < len(p.items)-1 { + p.invCursor++ + } + case "enter", "v", "right": + p.openView() + case "d": + if len(p.items) > 0 { + p.mode = modeConfirm + } + } + return nil +} + +func (p *evolvePanel) handleConfirmKey(msg tea.KeyMsg) tea.Cmd { + switch msg.String() { + case "y", "enter": + p.deleteSelected() + case "n", "esc": + p.mode = modeList + } + return nil +} + +// deleteSelected removes the focused item (skill or memory file) from disk, +// reloads the inventory, and returns to the list (or the form when it empties). +func (p *evolvePanel) deleteSelected() { + if p.invCursor >= len(p.items) { + p.mode = modeList + return + } + it := p.items[p.invCursor] + del := p.store.Delete + if it.kind == "memory" { + del = p.memStore.Delete + } + if del != nil { + if err := del(it.storeKey); err != nil { + p.actionErr = err + p.mode = modeList + return + } + } + p.reloadItems() + if len(p.items) == 0 { + p.mode = modeForm + } else { + p.mode = modeList + } +} + +func (p *evolvePanel) openView() { + if len(p.items) == 0 { + return + } + it := p.items[p.invCursor] + read := p.store.Read + if it.kind == "memory" { + read = p.memStore.Read + } + p.viewName = it.name + p.viewKind = it.kind + p.viewOffset = 0 + p.actionErr = nil + p.viewLines = nil + if read == nil { + p.actionErr = errPreviewUnavailable + return // stay in list; the inline error explains why + } + content, err := read(it.storeKey) + if err != nil { + p.actionErr = err + return + } + p.viewLines = splitLines(content) + p.mode = modeView +} + +func (p *evolvePanel) handleViewKey(msg tea.KeyMsg) tea.Cmd { + switch msg.String() { + case "esc", "q", "backspace", "left": + p.mode = modeList + case "up", "k": + p.viewOffset-- + case "down", "j": + p.viewOffset++ + case "pgup": + p.viewOffset -= viewPageStep + case "pgdown", " ": + p.viewOffset += viewPageStep + case "home", "g": + p.viewOffset = 0 + case "end", "G": + p.viewOffset = len(p.viewLines) // clamped to max in renderView + } + if p.viewOffset < 0 { + p.viewOffset = 0 + } + return nil +} + +// evolveRows is the /evolve config zone. Triggering is model-decided (the +// Evolve tool), so there are no cadence knobs: the skill checkboxes are pure +// permission gates on what a model-requested review may do to the skill set, +// and there is no separate enable toggle — skill-evolving is on when at least +// one action is allowed. learnedCount is shown on the "Learned" entry that +// drills into the inventory. +func evolveRows(learnedCount int) []configRow { + skillPerm := func(label string, get func(setting.SelfLearnSkills) bool, flip func(*setting.SelfLearnSkills)) configRow { + return configRow{ + kind: rowBool, + label: label, + indent: 2, + boolGetter: func(s *setting.SelfLearnSettings) bool { return get(s.Skills) }, + toggle: func(s *setting.SelfLearnSettings) { flip(&s.Skills) }, + } + } + return []configRow{ + { + kind: rowEntry, + entryID: "strategy", + label: "Strategy", + desc: "how to guide the learning", + indent: 1, + summary: func(s *setting.SelfLearnSettings) string { return strategySummary(s.Strategy) }, + }, + {kind: rowSpacer}, + {kind: rowSubHeader, label: "Skills", indent: 1}, + skillPerm("Create new skills", setting.SelfLearnSkills.AllowCreate, + func(s *setting.SelfLearnSkills) { s.DenyCreate = !s.DenyCreate }), + skillPerm("Update a skill", setting.SelfLearnSkills.AllowUpdate, + func(s *setting.SelfLearnSkills) { s.DenyUpdate = !s.DenyUpdate }), + skillPerm("Delete a skill", setting.SelfLearnSkills.AllowDelete, + func(s *setting.SelfLearnSkills) { s.DenyDelete = !s.DenyDelete }), + {kind: rowSpacer}, + {kind: rowSubHeader, label: "Memory", indent: 1}, + { + kind: rowBool, + label: "Enable memory", + indent: 2, + boolGetter: func(s *setting.SelfLearnSettings) bool { return s.Memory.Enabled }, + toggle: func(s *setting.SelfLearnSettings) { s.Memory.Enabled = !s.Memory.Enabled }, + }, + { + kind: rowInt, + label: "Max size", + unit: "KB", + indent: 2, + intGetter: func(s *setting.SelfLearnSettings) int { return s.Memory.ResolvedMaxKB() }, + intSetter: func(s *setting.SelfLearnSettings, v int) { s.Memory.MaxKB = v }, + intMin: 1, + intMax: setting.SelfLearnMaxMemoryKB, + footnote: func(v int) string { + return fmt.Sprintf("%d EN words / %d 中文字 (UTF-8)", v*180, v*340) + }, + }, + { + kind: rowText, + label: "Storage path", + indent: 2, + placeholder: "default (per-project store)", + strGetter: func(s *setting.SelfLearnSettings) string { return s.Memory.Path }, + strSetter: func(s *setting.SelfLearnSettings, v string) { s.Memory.Path = v }, + }, + {kind: rowSpacer}, + { + kind: rowEntry, + entryID: "learned", + label: "Learned", + desc: "skills & memory — browse & prune", + indent: 1, + summary: func(*setting.SelfLearnSettings) string { + if learnedCount == 0 { + return "none yet" + } + return strconv.Itoa(learnedCount) + }, + }, + {kind: rowSpacer}, + {kind: rowSave, label: "Save"}, + } +} diff --git a/internal/app/input/evolve_skills_view.go b/internal/app/input/evolve_skills_view.go new file mode 100644 index 00000000..4a8ab059 --- /dev/null +++ b/internal/app/input/evolve_skills_view.go @@ -0,0 +1,265 @@ +// Rendering for the /evolve panel: the config zone, the LEARNED inventory +// (skills + memory), and the full scrollable preview in modeView. Kept beside +// the panel's state/navigation (evolve_skills.go) so that file stays focused +// on the mode machine. +package input + +import ( + "errors" + "fmt" + "strings" + + "charm.land/lipgloss/v2" + + "github.com/genai-io/san/internal/app/kit" +) + +const ( + viewPageStep = 10 // lines per pgup/pgdown in the SKILL.md preview + recentZoneRows = 4 // max activity rows shown in the RECENT zone +) + +var errPreviewUnavailable = errors.New("preview unavailable") + +// Render dispatches by mode: the base config form, or one of the drill-in +// sub-views (Learning editor, learned-skills inventory, scrollable preview). +// height is the content-row budget handed down by the popup shell. +func (p *evolvePanel) Render(width, height int) string { + switch p.mode { + case modeStrategy: + return p.strategy.render(width, height) + case modeView: + return p.renderView(width, height) + case modeList, modeConfirm: + return p.renderLearned(width, height) + default: + return p.form.Render(width) + } +} + +// renderLearned is the drill-in inventory sub-view: the agent-created skills +// (browse / view / delete) with the recent-activity recap below. +func (p *evolvePanel) renderLearned(width, height int) string { + recent := p.renderRecent(width) + invBudget := max(3, height-strings.Count(recent, "\n")-1) + return p.renderInventory(width, invBudget) + recent +} + +// renderRecent draws the read-only RECENT zone: the last few skill-arm writes +// the reviewer made. Empty (no zone at all) when there's no activity yet, so an +// untouched panel isn't padded with a bare header. +func (p *evolvePanel) renderRecent(width int) string { + if len(p.events) == 0 { + return "" + } + var b strings.Builder + b.WriteString("\n") + b.WriteString(sectionRule("RECENT", width)) + b.WriteString("\n") + shown := min(len(p.events), recentZoneRows) + for _, e := range p.events[:shown] { + b.WriteString(renderRecentRow(e, width)) + b.WriteString("\n") + } + return b.String() +} + +// renderRecentRow formats one activity line: "· patched go-table-tests — note 2m ago". +func renderRecentRow(e LearnEvent, width int) string { + head := " " + recentVerbStyle.Render("· "+e.Verb+" ") + skillNameStyle.Render(e.Target) + ago := selflearnMutedStyle.Render(e.Ago) + note := "" + if e.Note != "" { + // Leave room for the right-aligned age plus a gap. + noteWidth := width - lipgloss.Width(head) - lipgloss.Width(e.Ago) - 6 + if noteWidth > 6 { + note = selflearnMutedStyle.Render(" — " + kit.TruncateText(e.Note, noteWidth)) + } + } + left := head + note + gap := max(width-lipgloss.Width(left)-lipgloss.Width(e.Ago), 1) + return left + strings.Repeat(" ", gap) + ago +} + +// renderInventory draws the learned list: agent-created skills then agent-written +// memory files, grouped by a SKILLS / MEMORY sub-header, windowed and +// cursor-aware, or an empty-state line when there's nothing. The "learned" +// header comes from the sub-view breadcrumb, so it isn't repeated here. +func (p *evolvePanel) renderInventory(width, budget int) string { + var b strings.Builder + + if len(p.items) == 0 { + b.WriteString(selflearnMutedStyle.Render(" Nothing learned yet.")) + b.WriteString("\n") + b.WriteString(p.inventoryError()) + return b.String() + } + + // Window the list around the cursor so a long inventory scrolls in place. + // Reserve rows for everything that renders besides the items themselves: + // one group sub-header per kind present, a possible error line, and the + // ↑/↓ overflow markers once the list scrolls. + reserved := 1 + countKindHeaders(p.items) + if len(p.items)+reserved > budget { + reserved += 2 + } + rows := max(1, budget-reserved) + start := 0 + if len(p.items) > rows && p.invCursor >= rows { + start = p.invCursor - rows + 1 + } + end := min(start+rows, len(p.items)) + if start > 0 { + b.WriteString(selflearnMutedStyle.Render(fmt.Sprintf(" ↑ %d more", start))) + b.WriteString("\n") + } + for i := start; i < end; i++ { + // A group sub-header opens each kind's block (skills, then memory). + if i == 0 || p.items[i].kind != p.items[i-1].kind { + b.WriteString(" " + selflearnSubHeaderStyle.Render(kindHeader(p.items[i].kind))) + b.WriteString("\n") + } + b.WriteString(p.renderItemRow(i, width)) + b.WriteString("\n") + } + if end < len(p.items) { + b.WriteString(selflearnMutedStyle.Render(fmt.Sprintf(" ↓ %d more", len(p.items)-end))) + b.WriteString("\n") + } + if p.mode == modeConfirm && p.invCursor < len(p.items) { + b.WriteString(skillConfirmStyle.Render( + fmt.Sprintf(" ⚠ Delete %q? ", p.items[p.invCursor].name)) + + selflearnMutedStyle.Render("y confirm · n cancel")) + b.WriteString("\n") + } + b.WriteString(p.inventoryError()) + return b.String() +} + +// kindHeader maps an item kind to its group sub-header label. +func kindHeader(kind string) string { + if kind == "memory" { + return "MEMORY" + } + return "SKILLS" +} + +// countKindHeaders returns how many group sub-headers the inventory renders — +// one per run of same-kind items (skills first, then memory, so at most two). +func countKindHeaders(items []learnedItem) int { + n := 0 + for i, it := range items { + if i == 0 || it.kind != items[i-1].kind { + n++ + } + } + return n +} + +func (p *evolvePanel) inventoryError() string { + if p.actionErr == nil { + return "" + } + return selflearnErrorStyle.Render(" ⚠ "+p.actionErr.Error()) + "\n" +} + +// renderItemRow formats one inventory row (skill or memory): cursor caret, name, +// a subtitle column (skill scope / memory summary), and — for skills — a truncated +// description, with an inline action hint on the focused row. +func (p *evolvePanel) renderItemRow(i, width int) string { + it := p.items[i] + // Reachable only from the modeList/modeConfirm render path. + focused := i == p.invCursor + + caret := strings.Repeat(" ", cursorWidth) + nameStyle := skillNameStyle + if focused { + caret = selflearnCursorStyle.Render("▸ ") + nameStyle = skillNameFocusStyle + } + + name := padRight(it.name, 22) + subtitle := padRight(it.subtitle, 9) + line := caret + nameStyle.Render(name) + selflearnMutedStyle.Render(subtitle) + + // Skills carry a description; it fills the rest of the row, leaving room for + // the focused-row action hint on the right. + if it.desc != "" { + used := cursorWidth + 22 + 9 + descWidth := width - used - 20 + if descWidth > 3 { + line += selflearnMutedStyle.Render(kit.TruncateText(it.desc, descWidth)) + } + } + // The action hint rides only the focused list row — during a delete confirm + // the prompt below the list carries the keys instead. + if focused && p.mode == modeList { + line += " " + skillActionHintStyle.Render("enter view · d del") + } + return line +} + +// renderView draws the scrollable, read-only SKILL.md preview: a title, a +// windowed slice of the file at viewOffset, and a line-position footer. +func (p *evolvePanel) renderView(width, height int) string { + var b strings.Builder + title := "SKILL.md · " + p.viewName + if p.viewKind == "memory" { + title = "MEMORY · " + p.viewName + } + b.WriteString(skillViewTitleStyle.Render(kit.TruncateText(title, width))) + b.WriteString("\n\n") + + win := max(3, height-3) + maxOffset := max(0, len(p.viewLines)-win) + if p.viewOffset > maxOffset { + p.viewOffset = maxOffset // clamp (idempotent — only ever lowers) + } + end := min(p.viewOffset+win, len(p.viewLines)) + for i := p.viewOffset; i < end; i++ { + b.WriteString(" " + kit.TruncateText(p.viewLines[i], width-2)) + b.WriteString("\n") + } + + if len(p.viewLines) > win { + b.WriteString("\n") + b.WriteString(selflearnMutedStyle.Render( + fmt.Sprintf(" line %d–%d of %d", p.viewOffset+1, end, len(p.viewLines)))) + } + return b.String() +} + +// ── small helpers ──────────────────────────────────────────────────────── + +// sectionRule renders "LABEL ─────…" — a sub-header-styled label with a faint +// rule filling the rest of the row. +func sectionRule(label string, width int) string { + up := strings.ToUpper(label) + ruleLen := max(width-len(up)-1, 1) + return selflearnSubHeaderStyle.Render(up) + " " + + popupRuleStyle.Render(strings.Repeat("─", ruleLen)) +} + +func splitLines(s string) []string { + return strings.Split(strings.ReplaceAll(s, "\r\n", "\n"), "\n") +} + +// padRight pads s with spaces to n runes, truncating (with an ellipsis) when +// it is already wider than n. +func padRight(s string, n int) string { + if r := []rune(s); len(r) < n { + return s + strings.Repeat(" ", n-len(r)) + } + return kit.TruncateText(s, n) +} + +var ( + skillNameStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text) + skillNameFocusStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text).Bold(true) + skillActionHintStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) + skillViewTitleStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text).Bold(true) + // Delete confirm keeps a warning hue — a rare, destructive-action moment + // worth one amber touch. + skillConfirmStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Warning).Bold(true) + recentVerbStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted) +) diff --git a/internal/app/input/evolve_strategy.go b/internal/app/input/evolve_strategy.go new file mode 100644 index 00000000..74b4c5d3 --- /dev/null +++ b/internal/app/input/evolve_strategy.go @@ -0,0 +1,114 @@ +// strategyEditor is the /evolve Strategy sub-view: a chromeless textarea +// seeded with the built-in learning strategy (or the user's saved override). +// On close its value collapses back to "" when left unchanged from the +// default, so the panel keeps reading "built-in" and Dirty() doesn't flag a +// no-op edit. Full-editable: the saved text replaces the built-in guidance in +// the reviewer prompt (the permission gate still applies). +package input + +import ( + "strings" + + "charm.land/bubbles/v2/textarea" + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + xansi "github.com/charmbracelet/x/ansi" + + "github.com/genai-io/san/internal/app/kit" +) + +type strategyEditor struct { + ta textarea.Model + deflt string // the built-in strategy, set once at construction + + // Render memo: the measured display height is recomputed only when the + // value, width, or row cap changed — the textarea measure-render is the + // priciest widget draw in the popup, and render runs every frame. + lastValue string + lastWidth int + lastMaxRows int + lastRows int +} + +func newStrategyEditor(deflt string) strategyEditor { + return strategyEditor{ta: newChromelessTextarea(), deflt: deflt} +} + +// open seeds the editor with the current override (or the built-in default +// when empty) and focuses it. +func (e *strategyEditor) open(current string) { + seed := current + if seed == "" { + seed = e.deflt + } + e.ta.SetValue(seed) + e.ta.CursorEnd() + e.ta.Focus() +} + +// value returns the override to persist: "" when the text matches the built-in +// default (so the arm stays "built-in"), otherwise the edited text. +func (e *strategyEditor) value() string { + v := strings.TrimRight(e.ta.Value(), "\n") + if strings.TrimSpace(v) == strings.TrimSpace(e.deflt) { + return "" + } + return v +} + +func (e *strategyEditor) handleKey(msg tea.KeyMsg) tea.Cmd { + var cmd tea.Cmd + e.ta, cmd = e.ta.Update(msg) + return cmd +} + +func (e *strategyEditor) insert(s string) { e.ta.InsertString(s) } + +// insertNewline adds a line break — bound to Alt/Shift+Enter so plain Enter is +// free to mean "save & back". +func (e *strategyEditor) insertNewline() { e.ta.InsertString("\n") } + +// render sizes the textarea to its CONTENT (not the whole panel) so a short +// strategy doesn't leave a wall of blank space, then adds a one-line reminder +// that the edit is staged until the panel is saved. `height` is only an upper +// bound: long text is capped there and scrolls. The content height is measured +// by rendering once at the cap and trimming the textarea's trailing +// end-of-buffer rows — which matches the widget's own soft-wrapping exactly +// (LineCount ignores wrapping) — and memoized until the value or geometry +// changes, so steady-state frames render the textarea once. +func (e *strategyEditor) render(width, height int) string { + e.ta.SetWidth(width) + maxRows := max(3, height-2) + if v := e.ta.Value(); v != e.lastValue || width != e.lastWidth || maxRows != e.lastMaxRows { + e.lastValue, e.lastWidth, e.lastMaxRows = v, width, maxRows + e.ta.SetHeight(maxRows) + measured := trimTrailingBlankLines(e.ta.View()) + e.lastRows = min(maxRows, max(3, strings.Count(measured, "\n")+1)) + } + e.ta.SetHeight(e.lastRows) + return e.ta.View() + "\n\n" + + strategyNoteStyle.Render("Replaces the built-in learning strategy — persisted when you Save the panel.") +} + +// trimTrailingBlankLines drops trailing blank lines (the textarea pads its box +// to the set height with end-of-buffer rows). Those rows carry the widget's +// styling, so the ANSI escapes must be stripped before the whitespace test — +// TrimSpace alone leaves the escape bytes and never sees the row as empty. +func trimTrailingBlankLines(s string) string { + lines := strings.Split(s, "\n") + n := len(lines) + for n > 0 && strings.TrimSpace(xansi.Strip(lines[n-1])) == "" { + n-- + } + return strings.Join(lines[:n], "\n") +} + +// strategySummary is the right-aligned value hint on the Strategy entry row. +func strategySummary(override string) string { + if strings.TrimSpace(override) != "" { + return "custom" + } + return "built-in" +} + +var strategyNoteStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted).Italic(true) diff --git a/internal/app/input/evolve_test.go b/internal/app/input/evolve_test.go new file mode 100644 index 00000000..6411910c --- /dev/null +++ b/internal/app/input/evolve_test.go @@ -0,0 +1,413 @@ +package input + +import ( + "errors" + "strings" + "testing" + + tea "charm.land/bubbletea/v2" + + "github.com/genai-io/san/internal/selflearn" +) + +// newTestEvolve builds an isolated /evolve popup with the Skills panel active. +// settings=nil so Enter() seeds the snapshot to the zero "feature off" +// baseline without touching disk. +func newTestEvolve() (*PanelPopup, *evolvePanel) { + c := NewEvolveSelector(EvolveDeps{}) + c.Enter(120, 40) + return &c, c.ActivePanel().(*evolvePanel) +} + +// TestSkillPanelCursorSkipsHeaders guards against nil-toggle panics: the +// cursor must never land on a section/sub-header / spacer row (toggle is nil → +// Space would crash). +func TestSkillPanelCursorSkipsHeaders(t *testing.T) { + c, p := newTestEvolve() + rows := p.form.rowsFn() + + if !rows[p.form.cursor].editable() { + t.Fatalf("initial cursor on non-editable row %d (%v)", p.form.cursor, rows[p.form.cursor].kind) + } + + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeySpace}) // toggle the first editable row + + for range len(rows) * 2 { + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) + if !rows[p.form.cursor].editable() { + t.Fatalf("down landed on non-editable row %d (%v)", p.form.cursor, rows[p.form.cursor].kind) + } + } + for range len(rows) * 2 { + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyUp}) + if !rows[p.form.cursor].editable() { + t.Fatalf("up landed on non-editable row %d (%v)", p.form.cursor, rows[p.form.cursor].kind) + } + } +} + +// TestEvolveSelectorActivatesAndDismisses confirms the popup flips on with +// Enter() and off when Esc is delivered through HandleKeypress. +func TestEvolveSelectorActivatesAndDismisses(t *testing.T) { + c, _ := newTestEvolve() + if !c.IsActive() { + t.Fatal("Enter should activate the popup") + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEscape}) + if c.IsActive() { + t.Fatal("Esc should deactivate") + } +} + +// TestSkillPanelTogglesBool walks the checkbox flow on Space. The first editable +// row is the Strategy entry, so step down onto the "Create new skills" +// permission (a plain checkbox — triggering is model-decided, no cadence). +func TestSkillPanelTogglesBool(t *testing.T) { + c, p := newTestEvolve() + if !p.form.snap.Skills.AllowCreate() { + t.Fatal("baseline: create should be allowed (Deny* zero value)") + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) // Strategy → Create + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeySpace}) + if p.form.snap.Skills.AllowCreate() { + t.Fatal("space should deny create") + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeySpace}) + if !p.form.snap.Skills.AllowCreate() { + t.Fatal("space should re-allow create") + } +} + +// TestPanelHasBothArms confirms the unified panel exposes the Skills permission +// gates and the Memory settings in one form (Create/Enable memory present). +func TestPanelHasBothArms(t *testing.T) { + _, p := newTestEvolve() + var haveCreate, haveMemory bool + for _, r := range p.form.rowsFn() { + switch r.label { + case "Create new skills": + haveCreate = true + case "Enable memory": + haveMemory = true + } + } + if !haveCreate || !haveMemory { + t.Fatalf("unified panel should show both arms; create=%v memory=%v", haveCreate, haveMemory) + } +} + +// TestSkillPanelRenderShowsValidationError confirms an invalid combination +// (create allowed but update denied) surfaces the inline error. +func TestSkillPanelRenderShowsValidationError(t *testing.T) { + c, p := newTestEvolve() + p.form.snap.Skills.DenyUpdate = true + out := c.Render() + if !strings.Contains(out, `"Create new skills" needs "Update a skill"`) { + t.Fatalf("Render should surface the validation error, got:\n%s", out) + } +} + +// TestEvolveSelectorRenderShowsSections confirms the "/evolve" header plus the +// SKILLS and MEMORY section sub-headers make it into the single-panel output. +func TestEvolveSelectorRenderShowsSections(t *testing.T) { + c, _ := newTestEvolve() + out := c.Render() + for _, want := range []string{"Self-Learning", "SKILLS", "MEMORY"} { + if !strings.Contains(out, want) { + t.Fatalf("render missing %q:\n%s", want, out) + } + } +} + +// TestSkillPanelScopeFlips toggles between user / project save targets with ←→. +func TestSkillPanelScopeFlips(t *testing.T) { + c, p := newTestEvolve() + if p.form.scope != "user" { + t.Fatalf("default scope: got %q, want user", p.form.scope) + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyRight}) + if p.form.scope != "project" { + t.Fatalf("after →: got %q", p.form.scope) + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyLeft}) + if p.form.scope != "user" { + t.Fatalf("after ←: got %q", p.form.scope) + } +} + +// TestSkillPanelRecentZone confirms the RECENT zone renders activity from both +// arms (one panel covers memory and skills now). +func TestSkillPanelRecentZone(t *testing.T) { + recent := func() []LearnEvent { + return []LearnEvent{ + {Kind: "skill", Verb: "patched", Target: "go-table-tests", Note: "trimmed examples", Ago: "2m ago"}, + {Kind: "memory", Verb: "added", Target: "user-prefs", Note: "x", Ago: "1h ago"}, + } + } + c := NewEvolveSelector(EvolveDeps{Recent: recent}) + c.Enter(120, 40) + p := c.ActivePanel().(*evolvePanel) + openLearnedView(&c, p) // RECENT lives in the learned-skills drill-in + out := c.Render() + for _, want := range []string{"RECENT", "go-table-tests", "2m ago", "user-prefs"} { + if !strings.Contains(out, want) { + t.Fatalf("RECENT zone missing %q:\n%s", want, out) + } + } +} + +// TestSkillPanelStrategyEditorFlow opens the Strategy editor (seeded with the +// built-in selflearn.DefaultStrategy), confirms it is modal, and that an +// unchanged default collapses to "" while an edited override persists (into +// the shared Strategy setting). +func TestSkillPanelStrategyEditorFlow(t *testing.T) { + c := NewEvolveSelector(EvolveDeps{}) + c.Enter(100, 40) + p := c.ActivePanel().(*evolvePanel) + + // Strategy is the first editable row, so enter opens it. + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) + if p.mode != modeStrategy || !p.Modal() { + t.Fatalf("enter on Strategy should open a modal editor, mode=%d", p.mode) + } + if p.Breadcrumb() != "strategy" { + t.Fatalf("breadcrumb = %q", p.Breadcrumb()) + } + + // Enter with the seed unchanged from the built-in default saves nothing. + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) + if p.mode != modeForm { + t.Fatal("enter should save the edit and return to the form") + } + if p.form.snap.Strategy != "" { + t.Fatalf("unchanged default should collapse to empty, got %q", p.form.snap.Strategy) + } + + // Reopen, type, Enter → a custom override is persisted. + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) + c.HandleKeypress(tea.KeyPressMsg{Code: '!', Text: "!"}) + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) + if p.form.snap.Strategy == "" { + t.Fatal("edited strategy should persist a custom override on enter") + } + if !p.form.Dirty() { + t.Fatal("a custom strategy override should mark the form dirty") + } + + // Esc discards: reopen, type, esc → the saved value is unchanged. + saved := p.form.snap.Strategy + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) + c.HandleKeypress(tea.KeyPressMsg{Code: 'X', Text: "X"}) + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEscape}) + if p.form.snap.Strategy != saved { + t.Fatalf("esc should discard the edit; got %q, want %q", p.form.snap.Strategy, saved) + } + if p.mode != modeForm { + t.Fatal("esc should return to the form") + } +} + +// TestMemoryPathEdit drives the inline free-text edit of the Memory storage path +// in the unified panel: navigate to it, enter to begin, type, enter to commit. +func TestMemoryPathEdit(t *testing.T) { + c, p := newTestEvolve() + + // Walk down to the "Storage path" row. + for range len(p.form.rowsFn()) { + if p.form.rowsFn()[p.form.cursor].label == "Storage path" { + break + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) // begin edit + if !p.form.editing { + t.Fatal("enter on Storage path should begin inline editing") + } + for _, ch := range "/tmp/mem" { + c.HandleKeypress(tea.KeyPressMsg{Code: ch, Text: string(ch)}) + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) // commit + if got := p.form.snap.Memory.Path; got != "/tmp/mem" { + t.Fatalf("storage path = %q, want /tmp/mem", got) + } + if !p.form.Dirty() { + t.Fatal("a set storage path should mark the form dirty") + } +} + +// ── Phase 2: learned-skills inventory ──────────────────────────────────── + +// fakeLearnedStore is a stateful in-memory LearnedSkillStore for tests: List +// reflects deletions, Read returns a multi-line stub SKILL.md. +func fakeLearnedStore(names ...string) LearnedSkillStore { + skills := make([]selflearn.SkillInfo, len(names)) + for i, n := range names { + skills[i] = selflearn.SkillInfo{Name: n, Level: "user", Description: n + " does a thing"} + } + return LearnedSkillStore{ + List: func() []selflearn.SkillInfo { return append([]selflearn.SkillInfo(nil), skills...) }, + Read: func(name string) (string, error) { + return "---\nname: " + name + "\n---\n\nline one\nline two\nline three\n", nil + }, + Delete: func(name string) error { + for i, s := range skills { + if s.Name == name { + skills = append(skills[:i], skills[i+1:]...) + return nil + } + } + return errors.New("no such skill") + }, + } +} + +func newTestEvolveWith(store LearnedSkillStore) (*PanelPopup, *evolvePanel) { + c := NewEvolveSelector(EvolveDeps{Learned: store}) + c.Enter(120, 40) + return &c, c.ActivePanel().(*evolvePanel) +} + +// fakeMemoryStore is a stateful in-memory LearnedMemoryStore for tests. +func fakeMemoryStore(topics ...string) LearnedMemoryStore { + mem := make([]LearnedMemory, len(topics)) + for i, t := range topics { + mem[i] = LearnedMemory{Topic: t, File: t + ".md", Summary: "3 lines"} + } + return LearnedMemoryStore{ + List: func() []LearnedMemory { return append([]LearnedMemory(nil), mem...) }, + Read: func(file string) (string, error) { return "memory: " + file + "\nline two\n", nil }, + Delete: func(file string) error { + for i, m := range mem { + if m.File == file { + mem = append(mem[:i], mem[i+1:]...) + return nil + } + } + return errors.New("no such memory file") + }, + } +} + +// TestLearnedDrillInCoversMemory confirms the "Learned" drill-in lists both +// skills and memory (grouped), and that memory items view + delete via the +// memory store. +func TestLearnedDrillInCoversMemory(t *testing.T) { + c := NewEvolveSelector(EvolveDeps{ + Learned: fakeLearnedStore("go-table-tests"), + Memory: fakeMemoryStore("debugging", "user-prefs"), + }) + c.Enter(120, 40) + p := c.ActivePanel().(*evolvePanel) + openLearnedView(&c, p) + + if len(p.items) != 3 { + t.Fatalf("drill-in should list 1 skill + 2 memory = 3 items, got %d: %+v", len(p.items), p.items) + } + // Skills come first, then memory. + if p.items[0].kind != "skill" || p.items[1].kind != "memory" { + t.Fatalf("expected skill then memory, got %q, %q", p.items[0].kind, p.items[1].kind) + } + // The render groups them under SKILLS / MEMORY sub-headers. + if out := c.Render(); !strings.Contains(out, "SKILLS") || !strings.Contains(out, "MEMORY") || !strings.Contains(out, "debugging") { + t.Fatalf("drill-in should render both groups + memory topics:\n%s", out) + } + + // Move onto a memory item and delete it via the memory store. + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) // skill → first memory + c.HandleKeypress(tea.KeyPressMsg{Code: 'd', Text: "d"}) + c.HandleKeypress(tea.KeyPressMsg{Code: 'y', Text: "y"}) + if len(p.items) != 2 { + t.Fatalf("deleting a memory item should leave 2, got %d", len(p.items)) + } + for _, it := range p.items { + if it.kind == "memory" && it.storeKey == "debugging.md" { + t.Fatal("debugging memory should have been deleted") + } + } +} + +// openLearnedView navigates from the config to the "Learned skills" entry and +// opens the inventory drill-in (modeList). +func openLearnedView(c *PanelPopup, p *evolvePanel) { + for range 20 { + if p.mode != modeForm { + return + } + if p.form.rowsFn()[p.form.cursor].entryID == "learned" { + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) + return + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) + } +} + +// TestSkillPanelOpensInventory confirms the "Learned skills" entry drills into +// the inventory list, ↑↓ navigate it, and esc returns to the config form. +func TestSkillPanelOpensInventory(t *testing.T) { + c, p := newTestEvolveWith(fakeLearnedStore("go-table-tests", "pr-triage")) + openLearnedView(c, p) + if p.mode != modeList || p.invCursor != 0 { + t.Fatalf("Learned skills entry should open the list at 0, got mode %d cursor %d", p.mode, p.invCursor) + } + if !p.Modal() { + t.Fatal("the inventory drill-in should be modal") + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) + if p.invCursor != 1 { + t.Fatalf("down in list should advance cursor, got %d", p.invCursor) + } + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEscape}) + if p.mode != modeForm { + t.Fatalf("esc should return to the config form, got mode %d", p.mode) + } + if !c.IsActive() { + t.Fatal("esc in the modal inventory must not dismiss the popup") + } +} + +// TestSkillPanelDeleteFlow drives d → confirm → y and checks the skill is +// removed from disk (the store) and the inventory reloads. +func TestSkillPanelDeleteFlow(t *testing.T) { + c, p := newTestEvolveWith(fakeLearnedStore("go-table-tests", "pr-triage")) + openLearnedView(c, p) + c.HandleKeypress(tea.KeyPressMsg{Code: 'd', Text: "d"}) + if p.mode != modeConfirm { + t.Fatalf("d should open the delete confirm, got mode %d", p.mode) + } + c.HandleKeypress(tea.KeyPressMsg{Code: 'y', Text: "y"}) + if p.mode != modeList { + t.Fatalf("y should delete and return to list, got mode %d", p.mode) + } + if len(p.items) != 1 || p.items[0].name != "pr-triage" { + t.Fatalf("delete should leave only pr-triage, got %+v", p.items) + } + if p.actionErr != nil { + t.Fatalf("unexpected action error: %v", p.actionErr) + } +} + +// TestSkillPanelViewScrollAndBack opens the preview, confirms it captures esc +// (modal) to return to the list rather than dismissing the popup. +func TestSkillPanelViewScrollAndBack(t *testing.T) { + c, p := newTestEvolveWith(fakeLearnedStore("go-table-tests")) + openLearnedView(c, p) + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEnter}) // open preview + if p.mode != modeView { + t.Fatalf("enter should open the preview, got mode %d", p.mode) + } + if len(p.viewLines) == 0 { + t.Fatal("preview should have loaded the SKILL.md lines") + } + if !p.Modal() { + t.Fatal("the preview should be modal") + } + // Esc is captured by the modal panel, so the popup stays open and returns + // to the list rather than dismissing. + c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyEscape}) + if !c.IsActive() { + t.Fatal("esc in the modal preview must not dismiss the popup") + } + if p.mode != modeList { + t.Fatalf("esc should return the preview to the list, got mode %d", p.mode) + } +} diff --git a/internal/app/input/model.go b/internal/app/input/model.go index f69b1249..a74d214a 100644 --- a/internal/app/input/model.go +++ b/internal/app/input/model.go @@ -59,8 +59,9 @@ type Model struct { Search SearchSelector Plugin PluginSelector Tool ToolSelector - Config ConfigSelector - Autopilot AutopilotSelector + Config PanelPopup // /config: appearance (+ future provider/permissions) + Evolve PanelPopup // /evolve: self-learning skills + memory + Autopilot AutopilotSelector // /autopilot: the session copilot // Selectors carrying ambient state (the picker is the .Selector field). Skill SkillState // + pending skill invocation @@ -114,6 +115,10 @@ type SelectorDeps struct { Setting *coresetting.Settings LoadDisabled func(userLevel bool) map[string]bool UpdateDisabled func(disabled map[string]bool, userLevel bool) error + // Evolve bundles the /evolve popup's dependencies: the live workspace + // source, the learned skill/memory stores, and the recent-activity + // accessor. See EvolveDeps. + Evolve EvolveDeps } func New(cwd string, width int, matchFunc suggest.Matcher, deps SelectorDeps) Model { @@ -139,6 +144,7 @@ func New(cwd string, width int, matchFunc suggest.Matcher, deps SelectorDeps) Mo Tool: NewToolSelector(deps.LoadDisabled, deps.UpdateDisabled), Config: NewConfigSelector(deps.Setting), Autopilot: NewAutopilotSelector(), + Evolve: NewEvolveSelector(deps.Evolve), } } diff --git a/internal/app/input/panel_form.go b/internal/app/input/panel_form.go new file mode 100644 index 00000000..2a5f3c01 --- /dev/null +++ b/internal/app/input/panel_form.go @@ -0,0 +1,589 @@ +// selfLearnForm is the config-zone form of the /evolve panel. It edits a +// working setting.SelfLearnSettings snapshot against an on-disk baseline, +// validates the §3.1 invariants inline, and writes to either the user-level +// or project-level settings file on Save. +// +// The owning panel supplies its row list via rowsFn; the snap, scope +// selector, cursor/edit machinery, and Save action live here. The panel +// renders additional zones (the Strategy editor, the Learned drill-in) +// around the form by composing rather than extending it. +package input + +import ( + "strconv" + "strings" + "unicode" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + + "github.com/genai-io/san/internal/app/kit" + "github.com/genai-io/san/internal/setting" +) + +// ConfigSavedMsg is emitted on a successful self-learning Save so the app can +// show a transient confirmation. SavedSelfLearn carries the snapshot that was +// just written so the consumer can compare it against the post-Reload +// effective state and detect cross-level overrides (settings merger ORs +// Enabled across user+project). +type ConfigSavedMsg struct { + Scope string + SavedSelfLearn setting.SelfLearnSettings +} + +type selfLearnForm struct { + // workspace returns the live cwd + settings service; read at Enter so a + // project reload is picked up. Nil (tests) seeds a zero snapshot. + workspace func() (string, *setting.Settings) + + // rowsFn returns the panel-specific rows (skill rows or memory rows). + rowsFn func() []configRow + + // snap is the working buffer; Save merges it back to disk. + snap setting.SelfLearnSettings + // baseline captures snap as it was at Enter() time so the form can flag + // unsaved edits in the top-right corner. + baseline setting.SelfLearnSettings + scope string // "user" | "project" + + cursor int + editing bool + editingBuffer string +} + +func (f *selfLearnForm) Enter() { + f.editing = false + f.editingBuffer = "" + f.snap = setting.SelfLearnSettings{} + if f.workspace != nil { + if _, settings := f.workspace(); settings != nil { + if data := settings.Snapshot(); data != nil { + f.snap = data.SelfLearn + } + } + } + f.baseline = f.snap + f.cursor = findEditable(f.rowsFn(), 0, +1, 0) +} + +// Dirty reports whether the working snapshot diverges from the disk +// baseline — the shell uses it to pin the "● unsaved" indicator. +func (f *selfLearnForm) Dirty() bool { return f.snap != f.baseline } + +// HandleKey handles one keypress. activated names the rowEntry the user +// pressed enter on ("" otherwise) so the owning panel can open the matching +// sub-view; done=true asks the shell to dismiss the popup (after Save). +func (f *selfLearnForm) HandleKey(msg tea.KeyMsg) (cmd tea.Cmd, done bool, activated string) { + if f.editing { + return f.handleEditingKey(msg), false, "" + } + rows := f.rowsFn() + if f.cursor >= len(rows) { + f.cursor = findEditable(rows, 0, +1, 0) + } + switch msg.String() { + case "up", "k": + f.cursor = findEditable(rows, f.cursor-1, -1, f.cursor) + case "down", "j": + f.cursor = findEditable(rows, f.cursor+1, +1, f.cursor) + case "left", "right": + // ←→ flips the save target; Tab is reserved by the shell for switching + // the skills ↔ memory tab. + if f.scope == "user" { + f.scope = "project" + } else { + f.scope = "user" + } + case "space": + // Space toggles the checkbox on a plain bool row or the checkbox half + // of a bool+int row. + if r := rows[f.cursor]; (r.kind == rowBool || r.kind == rowBoolInt) && r.toggle != nil { + r.toggle(&f.snap) + } + case "enter": + row := rows[f.cursor] + switch row.kind { + case rowBool: + if row.toggle != nil { + row.toggle(&f.snap) + } + case rowInt, rowBoolInt: + // Enter edits the number; Space toggles the checkbox (bool+int). + f.editing = true + f.editingBuffer = strconv.Itoa(row.intGetter(&f.snap)) + case rowText: + f.editing = true + f.editingBuffer = row.strGetter(&f.snap) + case rowEntry: + // The form doesn't own the sub-view; tell the panel to open it. + return nil, false, row.entryID + case rowSave: + cmd, done = f.save() + return cmd, done, "" + } + } + return nil, false, "" +} + +// save validates the working snapshot and persists it to the scoped settings +// file, emitting ConfigSavedMsg + done=true on success. A failed validation +// keeps the popup open (the inline error already shows why). +func (f *selfLearnForm) save() (tea.Cmd, bool) { + if err := f.snap.Validate(); err != nil { + return nil, false + } + userLevel := f.scope == "user" + if err := setting.UpdateSelfLearnAt(f.snap, userLevel); err != nil { + return nil, false + } + scope := f.scope + saved := f.snap + return func() tea.Msg { + return ConfigSavedMsg{Scope: scope, SavedSelfLearn: saved} + }, true +} + +func (f *selfLearnForm) handleEditingKey(msg tea.KeyMsg) tea.Cmd { + row := f.rowsFn()[f.cursor] + switch msg.String() { + case "esc": + f.editing = false + f.editingBuffer = "" + case "enter": + f.commitEdit(row) + f.editing = false + f.editingBuffer = "" + case "backspace": + if n := len(f.editingBuffer); n > 0 { + // Trim a whole final rune so multi-byte paths delete cleanly. + f.editingBuffer = trimLastRune(f.editingBuffer) + } + default: + f.appendEditText(row, msg.Key().Text) + } + return nil +} + +// commitEdit writes the edit buffer back to the snapshot: clamped int for +// rowInt, trimmed free text for rowText. +func (f *selfLearnForm) commitEdit(row configRow) { + switch row.kind { + case rowInt, rowBoolInt: + if v, err := strconv.Atoi(f.editingBuffer); err == nil { + v = min(max(v, row.intMin), row.intMax) + row.intSetter(&f.snap, v) + } + case rowText: + row.strSetter(&f.snap, strings.TrimSpace(f.editingBuffer)) + } +} + +// appendEditText appends typed input: digits only (capped at 4) for the numeric +// rows, any printable text (capped) for rowText. +func (f *selfLearnForm) appendEditText(row configRow, t string) { + switch row.kind { + case rowInt, rowBoolInt: + if len(t) == 1 && t[0] >= '0' && t[0] <= '9' && len(f.editingBuffer) < 4 { + f.editingBuffer += t + } + case rowText: + if t != "" && len(f.editingBuffer) < 512 { + f.editingBuffer += t + } + } +} + +// trimLastRune drops the final UTF-8 rune of s. +func trimLastRune(s string) string { + r := []rune(s) + return string(r[:len(r)-1]) +} + +// HintLine renders the bottom hint as monospace keycaps. +func (f *selfLearnForm) HintLine() string { + return keycap("↑↓") + " navigate " + + keycap("space") + " toggle " + + keycap("enter") + " edit/save " + + keycap("←→") + " scope" +} + +// Render draws the config-zone body. The arm's identity comes from the tab +// strip above, so there is no section header or rail here — just the scope +// control and a flat, calmly-spaced list of rows. +func (f *selfLearnForm) Render(width int) string { + rows := f.rowsFn() + validationErr := f.snap.Validate() + + var b strings.Builder + b.WriteString(f.renderScopeControl()) + b.WriteString("\n\n") + + for i, row := range rows { + switch row.kind { + case rowSubHeader: + // Align the label with the entry-row labels (which sit past a + // 2-col cursor gutter) so the section headers read as one level. + indentPad := strings.Repeat(" ", contentCol(row.indent)) + b.WriteString(indentPad + selflearnSubHeaderStyle.Render(strings.ToUpper(row.label))) + b.WriteString("\n") // breathing room under the sub-header + case rowSpacer: + // blank line + case rowSave: + b.WriteString(f.renderSaveRow(i, validationErr)) + case rowBool: + b.WriteString(f.renderBoolRow(i, row, width)) + case rowInt: + b.WriteString(f.renderIntRow(i, row, width)) + case rowBoolInt: + b.WriteString(f.renderBoolIntRow(i, row)) + case rowText: + b.WriteString(f.renderTextRow(i, row, width)) + case rowEntry: + b.WriteString(f.renderEntryRow(i, row, width)) + } + b.WriteString("\n") + } + + if validationErr != nil { + b.WriteString("\n") + b.WriteString(selflearnErrorStyle.Render("⚠ " + validationErr.Error())) + b.WriteString("\n") + } + return b.String() +} + +// renderScopeControl is a two-segment selector with the active segment +// as a filled pill. The "scope" label is dropped — the two visible +// segments (user / project) are self-explanatory. +func (f *selfLearnForm) renderScopeControl() string { + seg := func(name string) string { + if f.scope == name { + return selflearnScopeActiveStyle.Render(name) + } + return selflearnScopeIdleStyle.Render(name) + } + sep := selflearnMutedStyle.Render(" · ") + return seg("user") + sep + seg("project") +} + +// keycap renders a key label as a bg-filled pill so it doesn't read as +// a checkbox. The fill is the kit's neutral search-input gray so the +// keycap feels like a physical key cap, not a [ ] toggle. +func keycap(s string) string { + return selflearnKeycapStyle.Render(" " + capitalizeKeyLabel(s) + " ") +} + +// capitalizeKeyLabel title-cases each part of a key combo so hints read as +// proper key names: "ctrl+r" → "Ctrl+R", "enter" → "Enter". Arrow glyphs +// (↑↓, ←→) and other non-letters pass through unchanged. +func capitalizeKeyLabel(s string) string { + parts := strings.Split(s, "+") + for i, p := range parts { + if p == "" { + continue + } + r := []rune(p) + r[0] = unicode.ToUpper(r[0]) + parts[i] = string(r) + } + return strings.Join(parts, "+") +} + +// ── Row rendering ─────────────────────────────────────────────────────── + +// Layout columns. Every row's "content" (bracket for bool, value for int) +// starts at a column derived from its indent (indentStep cols per level); +// the cursor caret sits cursorWidth cols before that. Section headers go +// at indent 0 (col 0). Rows directly under a section: indent 1 (col 4). +// Sub-sections under those: header at indent 1, rows at indent 2 (col 8). +const ( + indentStep = 4 + cursorWidth = 2 +) + +// contentCol returns the column where the row's leftmost rendered content +// (bracket / value / save button) starts, for the given indent. +func contentCol(indent int) int { return indent * indentStep } + +func (f *selfLearnForm) cursorMark(i int) string { + if i == f.cursor { + return selflearnCursorStyle.Render("▸ ") + } + return strings.Repeat(" ", cursorWidth) +} + +// cursorPad returns the leading whitespace + cursor caret so the next +// glyph lands exactly at contentCol(indent). +func (f *selfLearnForm) cursorPad(i, indent int) string { + at := max(contentCol(indent)-cursorWidth, 0) + return strings.Repeat(" ", at) + f.cursorMark(i) +} + +func (f *selfLearnForm) renderBoolRow(i int, row configRow, _ int) string { + mark := "[ ]" + if row.boolGetter(&f.snap) { + mark = selflearnCheckStyle.Render("[✓]") + } + line := f.cursorPad(i, row.indent) + mark + " " + row.label + if row.note != "" { + line += " " + selflearnMutedStyle.Render(row.note) + } + return line +} + +// renderBoolIntRow draws a checkbox with an inline editable cadence on one line, +// e.g. "[✓] Create new skills every (10) iterations". Space toggles the box, +// Enter edits the number. The cadence is dimmed while the checkbox is off. +func (f *selfLearnForm) renderBoolIntRow(i int, row configRow) string { + on := row.boolGetter(&f.snap) + mark := "[ ]" + if on { + mark = selflearnCheckStyle.Render("[✓]") + } + value := strconv.Itoa(row.intGetter(&f.snap)) + if f.editing && i == f.cursor { + value = f.editingBuffer + "_" + } + cadence := selflearnMutedStyle.Render(row.intLead+" ") + valueChip(value) + if row.unit != "" { + cadence += " " + selflearnMutedStyle.Render(row.unit) + } + if !on { + cadence = selflearnFaintStyle.Render(row.intLead + " (" + value + ") " + row.unit) + } + return f.cursorPad(i, row.indent) + mark + " " + row.label + " " + cadence +} + +// renderEntryRow draws a drill-in section entry: "▸ STRATEGY how it guides … built-in". +// The label is an uppercase muted-bold header (the same level as a sub-header), +// with a muted description and a right-aligned value hint; enter-to-open is in +// the bottom hint. +func (f *selfLearnForm) renderEntryRow(i int, row configRow, width int) string { + left := f.cursorPad(i, row.indent) + selflearnSubHeaderStyle.Render(strings.ToUpper(row.label)) + if row.desc != "" { + left += " " + selflearnMutedStyle.Render(row.desc) + } + right := "" + if row.summary != nil { + right = selflearnEntrySummaryStyle.Render(row.summary(&f.snap)) + } + if right == "" { + return left + } + // Leave a 2-col margin before the card edge so the rail-prefixed row never + // spills the summary onto a wrapped line. + gap := max(width-lipgloss.Width(left)-lipgloss.Width(right)-2, 1) + return left + strings.Repeat(" ", gap) + right +} + +func (f *selfLearnForm) renderIntRow(i int, row configRow, _ int) string { + value := strconv.Itoa(row.intGetter(&f.snap)) + if f.editing && i == f.cursor { + value = f.editingBuffer + "_" + } + // Label-first phrase: "Run every (10) user turns" reads as a sentence. + // The label sits one bracket-width past where a bool row's "[" goes, + // so labels align with bool-row labels at the same indent. + labelStart := contentCol(row.indent) + 4 // 4 = bracket width "[ ] " + leftPad := strings.Repeat(" ", labelStart-cursorWidth) + line := leftPad + f.cursorMark(i) + row.label + " " + valueChip(value) + if row.unit != "" { + line += " " + selflearnMutedStyle.Render(row.unit) + } + if row.footnote != nil { + fn := row.footnote(row.intGetter(&f.snap)) + line += selflearnMutedStyle.Render(" ~ " + fn) + } + return line +} + +// renderTextRow draws a free-text row ("Storage path ") whose value is +// tail-anchored so the meaningful end of a long path stays visible; an empty, +// non-editing value shows the muted placeholder instead. +func (f *selfLearnForm) renderTextRow(i int, row configRow, width int) string { + labelStart := contentCol(row.indent) + 4 // align with bool-row labels + leftPad := strings.Repeat(" ", labelStart-cursorWidth) + head := leftPad + f.cursorMark(i) + row.label + " " + + editing := f.editing && i == f.cursor + value := row.strGetter(&f.snap) + if editing { + value = f.editingBuffer + } + + avail := max(width-lipgloss.Width(head)-1, 8) + if value == "" && !editing { + return head + selflearnMutedStyle.Render(tailTruncate(row.placeholder, avail)) + } + shown := tailTruncate(value, avail-1) // room for the edit caret + if editing { + shown += "_" + } + return head + selflearnValueStyle.Render(shown) +} + +// tailTruncate keeps the last n columns of s, prefixing "…" when clipped, so a +// long path shows its most-specific tail rather than its common root. +func tailTruncate(s string, n int) string { + if n <= 0 { + return "" + } + r := []rune(s) + if len(r) <= n { + return s + } + if n == 1 { + return "…" + } + return "…" + string(r[len(r)-(n-1):]) +} + +// valueChip wraps a numeric value in chip-style brackets so it reads as +// an editable input: muted parens around an accent-bold value. +func valueChip(value string) string { + return selflearnChipBracketStyle.Render("(") + + selflearnValueStyle.Render(value) + + selflearnChipBracketStyle.Render(")") +} + +func (f *selfLearnForm) renderSaveRow(i int, validationErr error) string { + style := selflearnSaveButtonStyle + if validationErr != nil { + style = selflearnSaveButtonDisabledStyle + } + btn := style.Render("Save") + tail := selflearnMutedStyle.Render(" or " + keycap("esc") + selflearnMutedStyle.Render(" to discard")) + return f.cursorPad(i, 1) + btn + tail +} + +// ── Row kinds and layout ──────────────────────────────────────────────── + +// rowKind discriminates the rendered row types: bool toggle, int with a +// clamped range, the save action, two header levels, a blank spacer, and +// the advanced-action hint. +type rowKind int + +const ( + rowBool rowKind = iota + rowInt + rowBoolInt // a checkbox with an inline editable number (Create + its cadence) + rowText // inline-edited free-text value (e.g. the memory storage path) + rowEntry // opens a panel-owned sub-view (e.g. the Steering Prompt editor) + rowSave + rowSubHeader // sub-section title (e.g. "Learn by") + rowSpacer // blank line +) + +// configRow is one renderable row. Fields unused by the row's kind stay zero. +type configRow struct { + kind rowKind + label string + toggle func(*setting.SelfLearnSettings) + boolGetter func(*setting.SelfLearnSettings) bool + intGetter func(*setting.SelfLearnSettings) int + intSetter func(*setting.SelfLearnSettings, int) + intMin int + intMax int + unit string // for rowInt/rowBoolInt — muted suffix after the value (e.g. "iterations", "KB") + intLead string // for rowBoolInt — muted word before the value chip (e.g. "every") + footnote func(int) string // for rowInt — optional muted inline footnote after the label + note string // for rowBool — optional muted inline hint after the label + indent int + // entryID identifies a rowEntry to its owning panel; desc/summary render + // the muted description and the right-aligned value hint (e.g. "built-in"). + entryID string + desc string + summary func(*setting.SelfLearnSettings) string + // strGetter/strSetter back a rowText; placeholder shows when the value is + // empty (e.g. "default (project store)"). + strGetter func(*setting.SelfLearnSettings) string + strSetter func(*setting.SelfLearnSettings, string) + placeholder string +} + +// editable reports whether the row can hold the cursor. Derived from kind: +// the actionable kinds (toggle, edit-in-place, text, sub-view entry, save) are +// editable; section headers, sub-headers, and spacers are not. +func (r configRow) editable() bool { + switch r.kind { + case rowBool, rowInt, rowBoolInt, rowText, rowEntry, rowSave: + return true + default: + return false + } +} + +// findEditable walks rows from start in direction step (+1 forward, +// −1 backward) until it hits an editable row. Returns fallback when +// no editable row is found in that direction — start ≥ len(rows) for +// the first-row lookup, the cursor itself for next/prev so navigation +// past the last actionable row stays put. +func findEditable(rows []configRow, start, step, fallback int) int { + for i := start; i >= 0 && i < len(rows); i += step { + if rows[i].editable() { + return i + } + } + return fallback +} + +// ── Styles ────────────────────────────────────────────────────────────── + +// Palette: teal (Focus) is the single accent, reserved for "focus / on" — the +// cursor caret and a checked box. Everything structural is grayscale +// (Text → Muted → TextDim → Faint). Error red is the only other hue, kept for +// the rare validation message. +var ( + // Headers stay calm muted-bold caps — headers are soft signposts, not + // shouting accents. + selflearnSubHeaderStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted).Bold(true) + + selflearnMutedStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted) + selflearnErrorStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Error) + + // The one accent: cursor caret and checked box. + selflearnCursorStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Focus).Bold(true) + selflearnCheckStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Focus) + + // Editable numeric value inside a "(…)" chip: plain bold text — the chip + // brackets carry the "editable" affordance, no color needed. + selflearnValueStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text).Bold(true) + + // Two-segment scope control: the active segment is a neutral filled pill, + // the inactive one a flat dim label. + selflearnScopeActiveStyle = lipgloss.NewStyle(). + Background(kit.SearchBg). + Foreground(kit.CurrentTheme.Text). + Bold(true). + Padding(0, 1) + selflearnScopeIdleStyle = lipgloss.NewStyle(). + Foreground(kit.CurrentTheme.TextDim). + Padding(0, 1) + + // Keycap pill — neutral gray bg + bold text so it reads as a physical key. + selflearnKeycapStyle = lipgloss.NewStyle(). + Background(kit.SearchBg). + Foreground(kit.CurrentTheme.Text). + Bold(true) + + // Faint wrapper for dimmed content (e.g. an unchecked action's cadence). + selflearnFaintStyle = lipgloss.NewStyle().Faint(true) + + // Save button — a neutral filled pill; the teal cursor caret marks focus. + // Dimmed when the snapshot fails validation. + selflearnSaveButtonStyle = lipgloss.NewStyle(). + Background(kit.SearchBg). + Foreground(kit.CurrentTheme.Text). + Bold(true). + Padding(0, 2) + selflearnSaveButtonDisabledStyle = lipgloss.NewStyle(). + Background(kit.SearchBg). + Foreground(kit.CurrentTheme.TextDim). + Padding(0, 2) + + // Chip-style brackets for editable values: "(10)" with dim parens. + selflearnChipBracketStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) + + // Right-aligned value hint on a sub-view entry row ("built-in" / "custom"). + selflearnEntrySummaryStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) +) diff --git a/internal/app/input/panel_popup.go b/internal/app/input/panel_popup.go new file mode 100644 index 00000000..9846776b --- /dev/null +++ b/internal/app/input/panel_popup.go @@ -0,0 +1,332 @@ +// Generic settings-popup shell. PanelPopup frames one or more sub-panels in a +// centered rounded card with a title lockup ("✦ Self-Learning"), a tab strip +// when multiple panels are registered, a faint hairline, and the active +// panel's body + hint. Each sub-panel implements Panel and owns its body. +// +// Two popups are built on this shell today: +// - /config → one Appearance panel (Provider / Permissions planned). +// - /evolve → one self-learning panel covering both arms (skills + memory). +// +// To add a panel to either, implement Panel and append it in the popup's +// constructor (NewConfigSelector / NewEvolveSelector). +package input + +import ( + "strings" + + tea "charm.land/bubbletea/v2" + "charm.land/lipgloss/v2" + + "github.com/genai-io/san/internal/app/kit" + "github.com/genai-io/san/internal/setting" +) + +// Panel is one popup sub-panel. +// +// Lifecycle: +// - Enter(): reset working state on (re)activation. +// - HandleKey(): handle one keypress; (cmd, done) — done=true asks the +// shell to dismiss the popup (e.g. after Save). +// - Render(width, height): the panel body, framed by the shell. height is +// the content rows available so a panel can window a scrollable region. +// - HintLine(): the muted bottom hint (e.g. "↑↓ navigate · …"). Shown +// under the body; the shell appends its own "· esc close". +type Panel interface { + Title() string + Enter() + HandleKey(msg tea.KeyMsg) (tea.Cmd, bool) + Render(width, height int) string + HintLine() string + // Dirty reports whether the panel has unsaved edits. The shell uses + // this to pin a "● unsaved" tag to the top-right of the header. + Dirty() bool +} + +// modalPanel is an optional Panel capability. While a panel is showing a +// sub-view (a scrollable preview, a text editor, a confirm prompt), it returns +// Modal()=true so the shell routes esc and ←→ to the panel via HandleKey — +// driving the sub-view — instead of dismissing the popup or switching tabs. +type modalPanel interface { + Modal() bool +} + +// breadcrumbPanel is an optional Panel capability: a panel inside a sub-view +// returns the sub-view's name so the shell can show "‹title› › ‹panel› › ‹sub›" +// in the header lockup (and hide the tab strip while you're deep in it). +type breadcrumbPanel interface { + Breadcrumb() string +} + +// PanelPopup is a centered rounded-card settings popup hosting one or more +// Panels behind a title lockup + tab strip. Adding a sibling panel is a +// one-liner in the popup's constructor. +type PanelPopup struct { + glyph string // title-lockup glyph, e.g. "✦" + title string // wordmark, e.g. "Self-Learning" + tagline string // muted tagline shown when not inside a sub-view + + panels []Panel + index int + + active bool + width int + height int +} + +// newPanelPopup wires a popup with its title lockup and set of panels. Order is +// preserved by the tab strip. +func newPanelPopup(glyph, title, tagline string, panels ...Panel) PanelPopup { + return PanelPopup{glyph: glyph, title: title, tagline: tagline, panels: panels} +} + +// NewConfigSelector builds the /config popup: Appearance today, with Provider +// / Permissions planned as sibling panels. +func NewConfigSelector(settings *setting.Settings) PanelPopup { + return newPanelPopup("⚙", "Config", "appearance & settings", newAppearancePanel(settings)) +} + +// Enter activates the popup with the first panel focused. +func (c *PanelPopup) Enter(width, height int) { + c.width = width + c.height = height + c.active = true + if c.index >= len(c.panels) { + c.index = 0 + } + if p := c.activePanel(); p != nil { + p.Enter() + } +} + +// IsActive implements the popup interface. +func (c *PanelPopup) IsActive() bool { return c.active } + +// HandleKeypress implements the popup interface. Esc dismisses the popup; +// Tab / Shift+Tab cycle the tab strip (skills ↔ memory); everything else — +// including ←/→, which the panels use for the user/project scope control — +// delegates to the active panel. +func (c *PanelPopup) HandleKeypress(msg tea.KeyMsg) tea.Cmd { + if !c.active { + return nil + } + p := c.activePanel() + if p == nil { + return nil + } + // A panel showing a sub-view (Modal) captures esc / Tab itself so those keys + // drive the sub-view rather than dismissing the popup or switching tabs. + // Otherwise the shell owns them. + if !panelIsModal(p) { + switch msg.String() { + case "esc": + c.active = false + return nil + case "tab", "shift+tab": + // Only intercept when there's more than one panel; otherwise fall + // through so the active panel can handle it. + if len(c.panels) > 1 { + step := 1 + if msg.String() == "shift+tab" { + step = -1 + } + c.index = (c.index + step + len(c.panels)) % len(c.panels) + c.panels[c.index].Enter() + return nil + } + } + } + cmd, done := p.HandleKey(msg) + if done { + c.active = false + } + return cmd +} + +// panelIsModal reports whether the panel currently owns a sub-view (see +// modalPanel). Panels that don't implement modalPanel are never modal. +func panelIsModal(p Panel) bool { + mp, ok := p.(modalPanel) + return ok && mp.Modal() +} + +// pasteSink is an optional Panel capability: a panel with an open text editor +// (the Strategy editor) accepts bracketed paste. Bracketed paste arrives as +// tea.PasteMsg, not a tea.KeyMsg, so it never reaches HandleKey. +type pasteSink interface { + HandlePaste(content string) +} + +// HandlePaste implements the app's pasteHandler: it routes bracketed paste to +// the active panel's text editor when one is open, so pasted strategy text +// lands in the textarea instead of leaking into the prompt behind the popup. +func (c *PanelPopup) HandlePaste(content string) tea.Cmd { + if !c.active || content == "" { + return nil + } + if ps, ok := c.activePanel().(pasteSink); ok { + ps.HandlePaste(content) + } + return nil +} + +// Render frames the active panel in a centered rounded card: the title lockup + +// tab strip, a faint hairline, the panel body, and the hint line. The column is +// built at exactly innerWidth and the card adds border+padding around it +// without re-setting a width, so nothing re-wraps. +func (c *PanelPopup) Render() string { + if !c.active || len(c.panels) == 0 { + return "" + } + p := c.activePanel() + w := c.innerWidth() + + header := c.renderHeader(w) + headerLines := strings.Count(header, "\n") + 1 + // Rows left for the body after the header, the hairline, the blank lines, + // the hint, and the card's border + padding + a screen margin. + bodyHeight := max(4, c.height-headerLines-12) + + rule := popupRuleStyle.Render(strings.Repeat("─", w)) + + body := p.Render(w, bodyHeight) + + var b strings.Builder + b.WriteString(header) + b.WriteString("\n") + b.WriteString(rule) + b.WriteString("\n\n") + b.WriteString(body) + b.WriteString("\n\n") + b.WriteString(c.renderHint(p.HintLine())) + + col := lipgloss.NewStyle().Width(w).Render(b.String()) + card := popupCardStyle.Render(col) + // Center on both axes so the card sits balanced in the terminal. + return lipgloss.Place(c.width, c.height-2, lipgloss.Center, lipgloss.Center, card) +} + +// innerWidth is the card's content column — a generous fill of the terminal so +// the panel reads as a confident, roomy card, capped so rows don't sprawl on an +// ultra-wide screen. The -10 leaves room for the card's border + padding and a +// screen margin. +func (c *PanelPopup) innerWidth() int { return min(max(c.width-10, 40), 120) } + +// ActivePanel returns the currently focused panel; nil when none are +// registered. Exported for tests. +func (c *PanelPopup) ActivePanel() Panel { return c.activePanel() } + +func (c *PanelPopup) activePanel() Panel { + if len(c.panels) == 0 { + return nil + } + return c.panels[c.index] +} + +// renderHeader renders the title lockup ("✦ Self-Learning" + tagline, or a +// "› panel › sub-view" breadcrumb when inside a sub-view), an "● unsaved" tag +// pinned right, and the tab strip below when more than one panel is registered. +func (c *PanelPopup) renderHeader(width int) string { + left := popupGlyphStyle.Render(c.glyph+" ") + popupTitleStyle.Render(c.title) + crumb := c.activeBreadcrumb() + switch { + case crumb != "": + // Multi-panel popups show "title › panel › sub-view"; a single-panel + // popup drops the redundant panel name ("title › sub-view"). + if len(c.panels) > 1 { + left += popupCrumbDimStyle.Render(" › ") + popupCrumbStyle.Render(c.activePanel().Title()) + } + left += popupCrumbDimStyle.Render(" › ") + popupCrumbStyle.Render(crumb) + case c.tagline != "": + left += popupTaglineStyle.Render(" " + c.tagline) + } + + line1 := left + if p := c.activePanel(); p != nil && p.Dirty() { + right := popupUnsavedDotStyle.Render("●") + " " + popupUnsavedTextStyle.Render("unsaved") + gap := max(width-lipgloss.Width(left)-lipgloss.Width(right), 1) + line1 = left + strings.Repeat(" ", gap) + right + } + + // Tab strip only at the top level (not while a sub-view breadcrumb shows). + if len(c.panels) > 1 && crumb == "" { + return line1 + "\n\n" + c.renderTabs() + } + return line1 +} + +// renderTabs draws the skills/memory tab strip as prominent uppercase pills — +// the active one filled in the accent, the rest a dim label. Roomier than the +// shared kit strip so the tabs read as the panel's primary navigation. +func (c *PanelPopup) renderTabs() string { + parts := make([]string, len(c.panels)) + for i, p := range c.panels { + label := strings.ToUpper(p.Title()) + if i == c.index { + parts[i] = popupTabActiveStyle.Render(label) + } else { + parts[i] = popupTabIdleStyle.Render(label) + } + } + return strings.Join(parts, " ") +} + +func (c *PanelPopup) activeBreadcrumb() string { + if bp, ok := c.activePanel().(breadcrumbPanel); ok { + return bp.Breadcrumb() + } + return "" +} + +// renderHint builds the bottom hint. A modal sub-view states its own keys, so +// the shell adds nothing; otherwise it appends "esc close" and, for a +// multi-panel popup, the tab-switch hint. +func (c *PanelPopup) renderHint(panelHint string) string { + if panelIsModal(c.activePanel()) { + return kit.HintLine(panelHint) + } + parts := []string{} + if panelHint != "" { + parts = append(parts, panelHint) + } + parts = append(parts, "esc close") + if len(c.panels) > 1 { + parts = append(parts, "tab switch panel") + } + return kit.HintLine(parts...) +} + +var ( + // Rounded card that frames the whole popup. + popupCardStyle = lipgloss.NewStyle(). + Border(lipgloss.RoundedBorder()). + BorderForeground(kit.CurrentTheme.Border). + Padding(1, 2) + + // Title lockup: the teal glyph is the one accent touch; the wordmark is + // plain bold text and the tagline a muted italic. + popupGlyphStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Focus) + popupTitleStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text).Bold(true) + popupTaglineStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted).Italic(true) + popupCrumbDimStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim) + popupCrumbStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Text).Bold(true) + + // Barely-there hairline under the header — a soft divider, not a heading. + popupRuleStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.TextDim).Faint(true) + + // "● unsaved" tag — muted, so it's noticeable without shouting in a hue. + popupUnsavedDotStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted).Bold(true) + popupUnsavedTextStyle = lipgloss.NewStyle().Foreground(kit.CurrentTheme.Muted) + + // Tab strip: the active tab is a filled teal pill (the one accent), + // uppercased and padded so the tabs read as the panel's primary nav; the + // rest are dim labels. + popupTabActiveStyle = lipgloss.NewStyle(). + Background(kit.CurrentTheme.Focus). + Foreground(kit.CurrentTheme.Background). + Bold(true). + Padding(0, 2) + popupTabIdleStyle = lipgloss.NewStyle(). + Foreground(kit.CurrentTheme.TextDim). + Bold(true). + Padding(0, 2) +) diff --git a/internal/app/input/preview_test.go b/internal/app/input/preview_test.go index f1c3fc64..a1885c4e 100644 --- a/internal/app/input/preview_test.go +++ b/internal/app/input/preview_test.go @@ -9,22 +9,19 @@ import ( tea "charm.land/bubbletea/v2" ) -// TestRenderPreview prints the panel without ANSI codes so the layout -// is visually inspectable via `go test -v -run TestRenderPreview`. +// TestRenderPreview prints the /evolve Skills panel without ANSI codes so the +// layout is visually inspectable via `go test -v -run TestRenderPreview`. func TestRenderPreview(t *testing.T) { - c := NewConfigSelector(nil) + c := NewEvolveSelector(EvolveDeps{}) c.Enter(80, 40) - // Toggle Memory's enable so the preview shows the heavy rail (┃) for - // the enabled section next to the dashed rail (╎) for the disabled one. - c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeySpace}) for range 3 { c.HandleKeypress(tea.KeyPressMsg{Code: tea.KeyDown}) } out := c.Render() ansi := regexp.MustCompile(`\x1b\[[0-9;]*[a-zA-Z]`) clean := ansi.ReplaceAllString(out, "") - if !strings.Contains(clean, "self-learning") { - t.Fatal("missing title") + if !strings.Contains(clean, "SKILLS") { + t.Fatal("missing skills tab") } if testing.Verbose() { fmt.Println("\n" + clean + "\n") diff --git a/internal/app/input/slash_command.go b/internal/app/input/slash_command.go index c44e5ea1..260a4104 100644 --- a/internal/app/input/slash_command.go +++ b/internal/app/input/slash_command.go @@ -122,6 +122,7 @@ func builtinCommandHandlers() map[string]slashCommandHandler { "config": (*SlashCommandController).handleConfigCommand, "autopilot": (*SlashCommandController).handleAutopilotCommand, "name": (*SlashCommandController).handleNameCommand, + "evolve": (*SlashCommandController).handleEvolveCommand, "selflearn-demo": (*SlashCommandController).handleSelflearnDemoCommand, } } @@ -342,11 +343,9 @@ func (c *SlashCommandController) handleResumeCommand(_ context.Context, _ string return "", nil, nil } -// handleConfigCommand opens the /config Self-Learning panel. Currently the -// panel only hosts the L1 config; a multi-panel sidebar (Provider / -// Permissions / Appearance / …) is planned and will be added by extending -// ConfigSelector with sibling panels — the slash command stays /config -// either way. +// handleConfigCommand opens the /config popup (Appearance today; Provider / +// Permissions are planned as sibling panels). Self-learning has moved out to +// its own /evolve popup. func (c *SlashCommandController) handleConfigCommand(_ context.Context, _ string) (string, tea.Cmd, error) { c.env.Input.Config.Enter(c.env.Width, c.env.Height) return "", nil, nil @@ -374,6 +373,13 @@ func (c *SlashCommandController) handleNameCommand(_ context.Context, args strin return fmt.Sprintf("Session renamed to: %s", name), nil, nil } +// handleEvolveCommand opens the /evolve popup — the self-learning +// configuration surface with Skills and Memory tabs. +func (c *SlashCommandController) handleEvolveCommand(_ context.Context, _ string) (string, tea.Cmd, error) { + c.env.Input.Evolve.Enter(c.env.Width, c.env.Height) + return "", nil, nil +} + // handleSelflearnDemoCommand drives the L1 status-bar indicator through // its phases (reviewing → recording actions → done) without firing a // real review fork. Hidden from the slash-command catalog; meant for diff --git a/internal/app/model.go b/internal/app/model.go index 53790ed0..b5564e74 100644 --- a/internal/app/model.go +++ b/internal/app/model.go @@ -31,20 +31,22 @@ import ( "github.com/genai-io/san/internal/app/hub" "github.com/genai-io/san/internal/app/input" "github.com/genai-io/san/internal/app/trigger" + "github.com/genai-io/san/internal/tool/evolve" ) const defaultWidth = 80 type model struct { // ── Sub-models (one per event source / concern) ───────────── - userInput input.Model // Source 1: user keyboard input - agentEventHub *hub.Hub // Source 2: inter-agent event routing (pure pub/sub) - mainEvents chan hub.Event // hub-side delivery chan; awaitMainEvent reads it - pendingMainEvents []hub.Event // events that arrived mid-stream, drained at OnTurnEnd - systemInput trigger.Model // Source 3: system events (cron/hooks/watcher) - conv conv.Model // Agent Outbox: conversation + output rendering - env env // Shared app state: provider, session, permission, plan, config - services services // Domain service singletons, injected at construction + userInput input.Model // Source 1: user keyboard input + agentEventHub *hub.Hub // Source 2: inter-agent event routing (pure pub/sub) + mainEvents chan hub.Event // hub-side delivery chan; awaitMainEvent reads it + pendingMainEvents []hub.Event // events that arrived mid-stream, drained at OnTurnEnd + systemInput trigger.Model // Source 3: system events (cron/hooks/watcher) + conv conv.Model // Agent Outbox: conversation + output rendering + env env // Shared app state: provider, session, permission, plan, config + services services // Domain service singletons, injected at construction + learnedStores *learnedStoreContext // live cwd/settings source for /evolve inventories // welcomePending marks the startup splash as not yet frozen into scrollback. // While set, the splash renders live above the input (visible from launch @@ -85,6 +87,23 @@ type model struct { // the mode indicator shows "thinking…" instead of a transcript notice. autopilotDeciding bool + // skillUsedThisTurn records whether the current turn invoked the Skill tool. + // It scopes the self-learning skills review: a skill-use turn weighs + // update/delete of that skill, a skill-free turn weighs create. Set in + // OnToolResult, read + cleared at OnTurnEnd. + skillUsedThisTurn bool + + // evolveRequestedThisTurn records whether the current turn called the + // Evolve tool — the model-decided self-learning trigger. Set in + // OnToolResult, read + cleared at OnTurnEnd. + evolveRequestedThisTurn bool + + // agentEvolveCaps records the self-learning capabilities the live agent's + // toolset was built with. ensureAgentSession compares them against the + // current settings on every turn start and rebuilds on drift — covering + // /evolve saves and external settings edits with one mechanism. + agentEvolveCaps evolve.Capabilities + // Streaming blocks render their markdown off the UI goroutine so a completed // block never stalls repaint. See flushState and model_scrollback.go. flush flushState diff --git a/internal/app/model_agent_events.go b/internal/app/model_agent_events.go index 56f78a09..a6f3dea2 100644 --- a/internal/app/model_agent_events.go +++ b/internal/app/model_agent_events.go @@ -16,6 +16,7 @@ import ( "github.com/genai-io/san/internal/core" "github.com/genai-io/san/internal/llm" "github.com/genai-io/san/internal/log" + "github.com/genai-io/san/internal/tool" ) func (m *model) OnTurnBegin() { @@ -71,6 +72,18 @@ func (m *model) OnAgentMessage(core.Message) tea.Cmd { } func (m *model) OnToolResult(tr core.ToolResult) *core.ToolResult { + // Track skill usage for the self-learning trigger: any Skill tool call this + // turn (even a failing one — a broken skill is a prime refine/retire + // candidate) flips the turn onto the update/delete review path. + if tr.ToolName == tool.ToolSkill { + m.skillUsedThisTurn = true + } + // The Evolve tool is the model-decided self-learning trigger: its call + // this turn queues a review at turn end. + if tr.ToolName == tool.ToolEvolve && !tr.IsError { + m.evolveRequestedThisTurn = true + } + sideEffect := m.services.Tool.PopSideEffect(tr.ToolCallID) if sideEffect != nil { m.applyToolSideEffects(tr.ToolName, sideEffect) @@ -93,11 +106,16 @@ func (m *model) OnTurnEnd(result core.Result) tea.Cmd { m.services.Tracker.Reset() } m.services.Agent.SetPluginRoot("") - // Forward to L1 self-learning. No-op when disabled; the reviewer gates - // on StopEndTurn internally so cancelled/interrupted turns are skipped. + // Forward to L1 self-learning with whether this turn used a skill and + // whether the model called Evolve (the model-decided trigger). No-op when + // disabled; the reviewer gates on StopEndTurn internally so cancelled / + // interrupted turns are skipped. Clear both flags for the next turn either + // way. if s := m.services.SelfLearn.session; s != nil { - s.reviewer.Observe(result) + s.reviewer.Observe(result, m.skillUsedThisTurn, m.evolveRequestedThisTurn) } + m.skillUsedThisTurn = false + m.evolveRequestedThisTurn = false log.QueueLog("OnTurnEnd: starting queueLen=%d", m.userInput.Queue.Len()) commitCmds := m.CommitMessages() diff --git a/internal/app/model_lifecycle.go b/internal/app/model_lifecycle.go index 4260a293..b1976f18 100644 --- a/internal/app/model_lifecycle.go +++ b/internal/app/model_lifecycle.go @@ -47,6 +47,7 @@ func newModel(opts setting.RunOptions) (*model, error) { func newBaseModel() model { svc := newServices() + learnedStores := newLearnedStoreContext(appCwd, svc.Setting) environment := newEnv(svc.LLM, appCwd, svc.Setting.IsGitRepo(appCwd)) if settings := svc.Setting.Snapshot(); settings != nil { environment.ApplyDefaultPermissionMode(settings.Permissions.DefaultMode, appCwd, svc.Setting.AllowBypass()) @@ -63,6 +64,12 @@ func newBaseModel() model { Setting: svc.Setting, LoadDisabled: svc.Setting.GetDisabledToolsAt, UpdateDisabled: svc.Setting.UpdateDisabledToolsAt, + Evolve: input.EvolveDeps{ + Workspace: learnedStores.Snapshot, + Learned: newLearnedSkillStore(learnedStores.Snapshot), + Memory: newLearnedMemoryStore(learnedStores.Snapshot), + Recent: newRecentLearnAccessor(svc.SelfLearn.Recent), + }, }), conv: conv.NewModel(defaultWidth), agentEventHub: hub.New(), @@ -70,6 +77,7 @@ func newBaseModel() model { systemInput: trigger.New(), env: environment, services: svc, + learnedStores: learnedStores, reviewerApprovals: new(atomic.Int64), reviewerEscalations: new(atomic.Int64), pendingDecisions: new(sync.Map), diff --git a/internal/app/selflearn.go b/internal/app/selflearn.go index 6d7fae0e..fb98e102 100644 --- a/internal/app/selflearn.go +++ b/internal/app/selflearn.go @@ -7,6 +7,8 @@ package app import ( "context" "fmt" + "os" + "path/filepath" "strings" "sync/atomic" "time" @@ -25,6 +27,7 @@ import ( "github.com/genai-io/san/internal/log" "github.com/genai-io/san/internal/selflearn" "github.com/genai-io/san/internal/setting" + "github.com/genai-io/san/internal/tool/evolve" ) // selfLearnDisableEnvSuffix is the env kill switch (§3.1) — mirrors Claude @@ -32,6 +35,41 @@ import ( // SAN_DISABLE_SELF_LEARN. const selfLearnDisableEnvSuffix = "DISABLE_SELF_LEARN" +// learnedStoreContext is the live workspace source behind the /evolve panel: +// workspace reloads replace both the cwd and the settings service, so the +// panel and its store accessors read them through here rather than capturing +// startup values. All access happens on the bubbletea update goroutine (panel +// key handling, reloadProjectServices), so no locking is needed. +type learnedStoreContext struct { + cwd string + settings *setting.Settings +} + +func newLearnedStoreContext(cwd string, settings *setting.Settings) *learnedStoreContext { + return &learnedStoreContext{cwd: cwd, settings: settings} +} + +func (c *learnedStoreContext) Snapshot() (string, *setting.Settings) { + return c.cwd, c.settings +} + +func (c *learnedStoreContext) Update(cwd string, settings *setting.Settings) { + c.cwd = cwd + c.settings = settings +} + +// resolveSelfLearnConfig is the shared runtime gate for both reviewer wiring +// and Evolve tool advertisement. The environment override is a hard disable; +// invalid settings likewise expose neither a reviewer nor a trigger tool. +func (m *model) resolveSelfLearnConfig() (selflearn.Config, error) { + if v := setting.Getenv(selfLearnDisableEnvSuffix); v == "1" || strings.EqualFold(v, "true") { + return selflearn.Config{}, nil + } + // Settings.SelfLearn() reads just the value-typed field — no whole-Data + // clone — so the per-turn drift check in ensureAgentSession stays cheap. + return selflearn.ResolveSettings(m.services.Setting.SelfLearn()) +} + // L1 review lifecycle event types published on agentEventHub. "started" is // an internal spinner wake-up consumed by onMainEvent; "done"/"failed" // surface as user-visible notices. @@ -44,23 +82,17 @@ const ( // wireSelfLearn builds the L1 Reviewer for the running session when ≥1 // arm is enabled. params is captured so the fork rebuilds an LLM client // with the same provider/model/max-tokens for prefix-cache parity -// (§6 invariant #2). pendingSend is the user content the caller is about -// to deliver — it is already in m.conv.Messages but has NOT been Observed -// yet, so SeedTurns must exclude it to keep the cadence beat honest. -func (m *model) wireSelfLearn(params agent.BuildParams, pendingSend string) { +// (§6 invariant #2). +func (m *model) wireSelfLearn(params agent.BuildParams) { // Tear down first — ensureAgentSession can re-enter via an agent // toggle (which calls Agent.Stop directly, bypassing StopAgentSession) // and would otherwise overwrite reviewCancel un-called, leaking the // context and pinning the old fork for up to forkDeadline. m.teardownSelfLearn() - // Env override wins: documented as the hard kill switch (§3.1). teardown - // above already cleared any prior session, so the disabled paths just return. - if v := setting.Getenv(selfLearnDisableEnvSuffix); v == "1" || strings.EqualFold(v, "true") { - return - } - snap := m.services.Setting.Snapshot() - cfg, err := selflearn.ResolveSettings(snap.SelfLearn) + // The shared resolver applies the env kill switch and validates settings so + // reviewer wiring and Evolve advertisement cannot disagree. + cfg, err := m.resolveSelfLearnConfig() if err != nil { log.Logger().Warn("self-learning config rejected at startup", zap.Error(err)) return @@ -82,8 +114,7 @@ func (m *model) wireSelfLearn(params agent.BuildParams, pendingSend string) { live := &atomic.Bool{} live.Store(true) - memStore := selflearn.NewMemoryStore(m.env.CWD, cfg.MemoryMaxChars) - skillMgr := selflearn.NewSkillManager(m.env.CWD, cfg.Perms) + memStore := selflearn.NewMemoryStore(m.env.CWD, cfg.MemoryMaxChars, cfg.MemoryPath) // Write observers feed the live spinner-tail and the post-pass recap. They // run on the fork goroutine and check `live` so a write landing after @@ -94,21 +125,17 @@ func (m *model) wireSelfLearn(params agent.BuildParams, pendingSend string) { if !live.Load() { return } - m.services.SelfLearn.Indicator.RecordAction(ReviewAction{ - Verb: verb, - Kind: kind, - Target: target, - Note: note, - }) + act := ReviewAction{Verb: verb, Kind: kind, Target: target, Note: note} + m.services.SelfLearn.Indicator.RecordAction(act) + // Also feed the persistent recent-activity log behind the /evolve + // RECENT zones (the indicator's copy is drained after each pass). + m.services.SelfLearn.Recent.Add(act, time.Now()) } memStore.SetWriteObserver(func(action, topic, note string) { recordAction("memory", memoryVerb(action), memoryTopicName(topic), note) }) - skillMgr.SetWriteObserver(func(action, skillName, note string) { - recordAction("skill", skillVerb(action), skillName, note) - }) - review := func(kinds selflearn.ReviewKind, snapshot []core.Message) { + review := func(kinds selflearn.ReviewKind, skillPerms selflearn.SkillPermissions, snapshot []core.Message) { // Liveness checks before any UI mutation — a teardown race must // not flash "evolving → evolved" on a session the user just killed. // Active() guards the macro window; the per-phase live.Load() checks @@ -124,6 +151,16 @@ func (m *model) wireSelfLearn(params agent.BuildParams, pendingSend string) { if !live.Load() { return } + + // The skill manager is scoped to exactly the actions the trigger allowed + // this pass (create on a skill-free turn, update/delete on a skill-use + // turn), so the reviewer can't take an action that wasn't triggered. All + // actions remain agent-created only. + skillMgr := selflearn.NewSkillManager(m.env.CWD, skillPerms) + skillMgr.SetWriteObserver(func(action, skillName, note string) { + recordAction("skill", skillVerb(action), skillName, note) + }) + m.services.SelfLearn.Indicator.BeginReview() m.publishSelfLearnStarted(kinds) @@ -140,12 +177,13 @@ func (m *model) wireSelfLearn(params agent.BuildParams, pendingSend string) { forkSessionID = rec.SessionID() } fc := selflearn.ForkConfig{ - LLM: client, - System: sys, - CWD: m.env.CWD, - Memory: memStore, - Skills: skillMgr, - OnEvent: forkOnEvent, + LLM: client, + System: sys, + CWD: m.env.CWD, + Memory: memStore, + Skills: skillMgr, + Strategy: cfg.Strategy, + OnEvent: forkOnEvent, } llmSummary, runErr := selflearn.RunReview(reviewCtx, fc, kinds, snapshot) // Re-check live AFTER the RunReview return. This is the macro @@ -191,7 +229,6 @@ func (m *model) wireSelfLearn(params agent.BuildParams, pendingSend string) { } r := selflearn.New(cfg, review) - r.SeedTurns(countUserTurns(m.conv.Messages, pendingSend)) m.services.SelfLearn.session = &selfLearnSession{ reviewer: r, cancel: reviewCancel, @@ -248,12 +285,11 @@ func (m *model) runSelfLearnDemo() { }() } -// notifySelfLearnOverride detects when a /config save was silently -// overridden by the OTHER settings level (the merger ORs the Enabled -// flags across user+project so disabling at one level cannot turn off an -// arm that the other level enabled). Surfaces a notice so the user -// learns about the override instead of seeing a "saved" confirmation -// while the behavior stays unchanged. +// notifySelfLearnOverride detects when a /evolve save was overridden by the +// other settings level. Memory enablement and each skill action are compared +// separately because the merger combines their safety gates independently. +// Surfaces a notice instead of leaving the user with only a misleading saved +// confirmation while the requested behavior stays unchanged. func (m *model) notifySelfLearnOverride(msg input.ConfigSavedMsg) { snap := m.services.Setting.Snapshot() if snap == nil { @@ -263,8 +299,14 @@ func (m *model) notifySelfLearnOverride(msg input.ConfigSavedMsg) { if msg.SavedSelfLearn.Memory.Enabled != snap.SelfLearn.Memory.Enabled { arms = append(arms, "Memory") } - if msg.SavedSelfLearn.Skills.Enabled != snap.SelfLearn.Skills.Enabled { - arms = append(arms, "Skills") + if msg.SavedSelfLearn.Skills.AllowCreate() != snap.SelfLearn.Skills.AllowCreate() { + arms = append(arms, "Skill create") + } + if msg.SavedSelfLearn.Skills.AllowUpdate() != snap.SelfLearn.Skills.AllowUpdate() { + arms = append(arms, "Skill update") + } + if msg.SavedSelfLearn.Skills.AllowDelete() != snap.SelfLearn.Skills.AllowDelete() { + arms = append(arms, "Skill delete") } if len(arms) == 0 { return @@ -274,7 +316,40 @@ func (m *model) notifySelfLearnOverride(msg input.ConfigSavedMsg) { other = "user" } m.conv.AddNotice("Note: " + strings.Join(arms, " and ") + - " enabled state is overridden by " + other + "-level settings") + " setting is overridden by " + other + "-level settings") +} + +// newLearnedSkillStore builds the /evolve inventory accessors over the live +// workspace source. Each call constructs a short-lived SkillManager doing a +// fresh disk scan (the reviewer mutates the skill dirs mid-session, so a +// cached view would go stale). This is a human-initiated surface, so it uses +// full action permissions — the reviewer's Deny* gates govern the autonomous +// loop, not the user's own deletions — and only ever surfaces agent-created +// skills. +func newLearnedSkillStore(source func() (string, *setting.Settings)) input.LearnedSkillStore { + mgr := func() *selflearn.SkillManager { + cwd, _ := source() + return selflearn.NewSkillManager(cwd, selflearn.AllowAllSkillActions()) + } + return input.LearnedSkillStore{ + List: func() []selflearn.SkillInfo { + var out []selflearn.SkillInfo + for _, s := range mgr().Inventory() { + if !s.Editable() { + continue // agent-created only; user-authored skills live in /skills + } + out = append(out, s) + } + return out + }, + Read: func(name string) (string, error) { + return mgr().Read(name) + }, + Delete: func(name string) error { + _, err := mgr().Delete(name, "removed via /evolve panel") + return err + }, + } } // teardownSelfLearn unwires the current L1 reviewer: cancels the @@ -291,6 +366,111 @@ func (m *model) teardownSelfLearn() { m.services.SelfLearn.session = nil } +// newLearnedMemoryStore builds the /evolve "Learned" memory accessors over the +// resolved auto-memory dir (honoring a configured memory path, read live from +// settings): list the agent-written .md files, read one, delete one. Read/Delete +// are guarded against path traversal. +func newLearnedMemoryStore(source func() (string, *setting.Settings)) input.LearnedMemoryStore { + dir := func() string { + cwd, settings := source() + var path string + if settings != nil { + if snap := settings.Snapshot(); snap != nil { + path = snap.SelfLearn.Memory.Path + } + } + return system.ResolveAutoMemoryDir(cwd, path) + } + return input.LearnedMemoryStore{ + List: func() []input.LearnedMemory { + d := dir() + entries, err := os.ReadDir(d) + if err != nil { + return nil + } + var out []input.LearnedMemory + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".md") { + continue + } + topic := name + if name != system.AutoMemoryIndexName { + topic = strings.TrimSuffix(name, ".md") + } + out = append(out, input.LearnedMemory{ + Topic: topic, + File: name, + Summary: memoryFileSummary(filepath.Join(d, name)), + }) + } + return out + }, + Read: func(file string) (string, error) { + if !safeMemoryFile(file) { + return "", fmt.Errorf("invalid memory file: %q", file) + } + b, err := os.ReadFile(filepath.Join(dir(), file)) + return string(b), err + }, + Delete: func(file string) error { + if !safeMemoryFile(file) { + return fmt.Errorf("invalid memory file: %q", file) + } + return os.Remove(filepath.Join(dir(), file)) + }, + } +} + +// safeMemoryFile guards Read/Delete against path traversal: the file must be a +// plain .md base name inside the memory dir. +func safeMemoryFile(file string) bool { + return file != "" && filepath.Base(file) == file && strings.HasSuffix(file, ".md") +} + +// memoryFileSummary returns a short "N lines" summary for an inventory row. +func memoryFileSummary(path string) string { + b, err := os.ReadFile(path) + if err != nil || len(b) == 0 { + return "empty" + } + n := strings.Count(string(b), "\n") + if !strings.HasSuffix(string(b), "\n") { + n++ + } + return fmt.Sprintf("%d lines", n) +} + +// selfLearnCapabilities returns the enabled self-learning capabilities, used +// to build (and gate) the Evolve trigger tool. The zero value — every skill +// action denied and memory off — means "self-learning off", so no tool is +// injected. Shares resolveSelfLearnConfig with wireSelfLearn so the advertised +// tool and the wired reviewer can never disagree (env kill switch, invalid +// settings). +func (m *model) selfLearnCapabilities() evolve.Capabilities { + cfg, err := m.resolveSelfLearnConfig() + if err != nil { + return evolve.Capabilities{} + } + return evolve.Capabilities{ + CreateSkills: cfg.Skills.AllowCreate, + UpdateSkills: cfg.Skills.AllowUpdate, + DeleteSkills: cfg.Skills.AllowDelete, + WriteMemory: cfg.MemoryEnabled, + } +} + +// selfLearnExtraTools returns the Evolve trigger tool for the main agent's +// toolset — the only conditional extra tool today. Nil when self-learning is +// off, so the tool is absent entirely. +func (m *model) selfLearnExtraTools() []core.ToolSchema { + caps := m.selfLearnCapabilities() + if !caps.Active() { + return nil + } + return []core.ToolSchema{evolve.Schema(caps)} +} + // handleSelflearnTick advances the indicator and schedules the next tick // at the cadence Tick returns (spinner interval while reviewing; one // deadline tick during done/failed hold). Returns nil when idle. @@ -315,29 +495,6 @@ func memoryTopicName(file string) string { return strings.TrimSuffix(file, ".md") } -// countUserTurns counts user messages already Observed by the reviewer -// so the memory arm resumes on the right cadence beat after session -// restore (§6 invariant #8). A trailing pendingSend match is excluded — -// the submit path Appends the message before ensureAgentSession runs, so -// without this guard the seed double-counts the in-flight turn that -// Observe is about to increment. -func countUserTurns(msgs []core.ChatMessage, pendingSend string) int { - end := len(msgs) - if pendingSend != "" && end > 0 { - last := msgs[end-1] - if last.Role == core.RoleUser && last.Content == pendingSend { - end-- - } - } - n := 0 - for i := 0; i < end; i++ { - if msgs[i].Role == core.RoleUser { - n++ - } - } - return n -} - // publishSelfLearnSummary posts the post-pass recap into the conversation // flow. Recap goes in Subject (display-only Notice); routing it through // Data would re-submit it to the LLM and break the §6 out-of-band promise. diff --git a/internal/app/selflearn_recent.go b/internal/app/selflearn_recent.go new file mode 100644 index 00000000..75bfff6e --- /dev/null +++ b/internal/app/selflearn_recent.go @@ -0,0 +1,90 @@ +package app + +import ( + "fmt" + "sync" + "time" + + "github.com/genai-io/san/internal/app/input" +) + +// recentLearnCap bounds the rolling self-learning activity log surfaced in the +// /evolve RECENT zones. Older events fall off the front. +const recentLearnCap = 20 + +// RecentLearnEvent is one logged self-learning write plus the wall-clock time +// it landed, for the RECENT activity recap. +type RecentLearnEvent struct { + ReviewAction + At time.Time +} + +// RecentLearnLog is a small mutex-guarded ring of recent self-learning writes. +// The per-session write observers append to it; the /evolve panels read a +// snapshot at open time. It outlives any one session (allocated once at +// services construction), so the recap survives /clear like the Indicator does. +type RecentLearnLog struct { + mu sync.Mutex + events []RecentLearnEvent +} + +// NewRecentLearnLog returns an empty log. +func NewRecentLearnLog() *RecentLearnLog { return &RecentLearnLog{} } + +// Add appends one event, evicting the oldest past the cap. at is the moment it +// landed; the panel humanizes it to "2m ago" at read time. +func (l *RecentLearnLog) Add(act ReviewAction, at time.Time) { + l.mu.Lock() + defer l.mu.Unlock() + l.events = append(l.events, RecentLearnEvent{ReviewAction: act, At: at}) + if len(l.events) > recentLearnCap { + l.events = l.events[len(l.events)-recentLearnCap:] + } +} + +// recent returns the logged events newest first (the ring is already capped +// at recentLearnCap by Add). +func (l *RecentLearnLog) recent() []RecentLearnEvent { + l.mu.Lock() + defer l.mu.Unlock() + out := make([]RecentLearnEvent, 0, len(l.events)) + for i := len(l.events) - 1; i >= 0; i-- { + out = append(out, l.events[i]) + } + return out +} + +// newRecentLearnAccessor returns the /evolve RECENT-zone accessor: a snapshot +// of the log's events as input-layer DTOs with their age humanized against the +// call time. +func newRecentLearnAccessor(log *RecentLearnLog) func() []input.LearnEvent { + return func() []input.LearnEvent { + now := time.Now() + evs := log.recent() + out := make([]input.LearnEvent, 0, len(evs)) + for _, e := range evs { + out = append(out, input.LearnEvent{ + Kind: e.Kind, + Verb: e.Verb, + Target: e.Target, + Note: e.Note, + Ago: humanizeAgo(now.Sub(e.At)), + }) + } + return out + } +} + +// humanizeAgo renders a duration as a compact "… ago" clause for the recap. +func humanizeAgo(d time.Duration) string { + switch { + case d < time.Minute: + return "just now" + case d < time.Hour: + return fmt.Sprintf("%dm ago", int(d.Minutes())) + case d < 24*time.Hour: + return fmt.Sprintf("%dh ago", int(d.Hours())) + default: + return fmt.Sprintf("%dd ago", int(d.Hours()/24)) + } +} diff --git a/internal/app/selflearn_review_fixes_test.go b/internal/app/selflearn_review_fixes_test.go new file mode 100644 index 00000000..6385bf3c --- /dev/null +++ b/internal/app/selflearn_review_fixes_test.go @@ -0,0 +1,150 @@ +package app + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/genai-io/san/internal/app/conv" + "github.com/genai-io/san/internal/app/input" + "github.com/genai-io/san/internal/core" + "github.com/genai-io/san/internal/core/system" + "github.com/genai-io/san/internal/hook" + "github.com/genai-io/san/internal/setting" + "github.com/genai-io/san/internal/tool" +) + +func TestFailedEvolveResultDoesNotRequestSelfLearn(t *testing.T) { + m := &model{services: services{ + Tool: tool.NewRegistry(), + Hook: hook.NewEngine(setting.NewData(), "", t.TempDir(), ""), + }} + + m.OnToolResult(core.ToolResult{ToolName: tool.ToolEvolve, IsError: true}) + if m.evolveRequestedThisTurn { + t.Fatal("failed Evolve result requested autonomous learning") + } + + m.OnToolResult(core.ToolResult{ToolName: tool.ToolEvolve}) + if !m.evolveRequestedThisTurn { + t.Fatal("successful Evolve result did not request learning") + } +} + +func TestSelfLearnCapabilitiesHonorHardDisable(t *testing.T) { + m := &model{services: services{Setting: &setting.Settings{}}} + + t.Setenv("SAN_DISABLE_SELF_LEARN", "") + if caps := m.selfLearnCapabilities(); !caps.Active() { + t.Fatal("default skill permissions should advertise Evolve") + } + + t.Setenv("SAN_DISABLE_SELF_LEARN", "1") + if caps := m.selfLearnCapabilities(); caps.Active() { + t.Fatalf("hard-disabled self-learning advertised Evolve: %+v", caps) + } +} + +func TestSelfLearnCapabilitiesRejectInvalidSettings(t *testing.T) { + home := t.TempDir() + cwd := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + writeTestFile(t, filepath.Join(cwd, ".san", "settings.json"), + `{"selfLearn":{"skills":{"enabled":true,"denyUpdate":true}}}`) + + settings := &setting.Settings{} + if err := settings.Reload(cwd); err != nil { + t.Fatal(err) + } + m := &model{services: services{Setting: settings}} + if caps := m.selfLearnCapabilities(); caps.Active() { + t.Fatalf("invalid settings advertised Evolve: %+v", caps) + } +} + +func TestNotifySelfLearnOverrideChecksIndividualSkillActions(t *testing.T) { + home := t.TempDir() + cwd := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + writeTestFile(t, filepath.Join(home, ".san", "settings.json"), + `{"selfLearn":{"skills":{"enabled":true,"denyCreate":true}}}`) + writeTestFile(t, filepath.Join(cwd, ".san", "settings.json"), + `{"selfLearn":{"skills":{"enabled":true}}}`) + + settings := &setting.Settings{} + if err := settings.Reload(cwd); err != nil { + t.Fatal(err) + } + m := &model{ + services: services{Setting: settings}, + conv: conv.NewModel(80), + } + m.notifySelfLearnOverride(input.ConfigSavedMsg{ + Scope: "project", + SavedSelfLearn: setting.SelfLearnSettings{Skills: setting.SelfLearnSkills{}}, + }) + + if len(m.conv.Messages) != 1 || !strings.Contains(m.conv.Messages[0].Content, "Skill create") { + t.Fatalf("per-action override notice missing: %+v", m.conv.Messages) + } +} + +func TestLearnedStoresFollowLiveWorkspace(t *testing.T) { + home := t.TempDir() + cwdA := t.TempDir() + cwdB := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + + writeTestFile(t, filepath.Join(cwdA, ".san", "settings.json"), + `{"selfLearn":{"memory":{"path":"memory-a"},"skills":{"enabled":true}}}`) + writeTestFile(t, filepath.Join(cwdB, ".san", "settings.json"), + `{"selfLearn":{"memory":{"path":"memory-b"},"skills":{"enabled":true}}}`) + writeTestFile(t, filepath.Join(cwdA, ".san", "skills", "skill-a", "SKILL.md"), + "---\ndescription: from a\norigin: agent-created\n---\n# A\n") + writeTestFile(t, filepath.Join(cwdB, ".san", "skills", "skill-b", "SKILL.md"), + "---\ndescription: from b\norigin: agent-created\n---\n# B\n") + writeTestFile(t, filepath.Join(cwdA, "memory-a", system.AutoMemoryIndexName), "from a\n") + writeTestFile(t, filepath.Join(cwdB, "memory-b", "topic-b.md"), "from b\n") + + settingsA := &setting.Settings{} + if err := settingsA.Reload(cwdA); err != nil { + t.Fatal(err) + } + source := newLearnedStoreContext(cwdA, settingsA) + skills := newLearnedSkillStore(source.Snapshot) + memory := newLearnedMemoryStore(source.Snapshot) + assertLearnedStoreNames(t, skills, memory, "skill-a", system.AutoMemoryIndexName) + + settingsB := &setting.Settings{} + if err := settingsB.Reload(cwdB); err != nil { + t.Fatal(err) + } + source.Update(cwdB, settingsB) + assertLearnedStoreNames(t, skills, memory, "skill-b", "topic-b.md") +} + +func assertLearnedStoreNames(t *testing.T, skills input.LearnedSkillStore, memory input.LearnedMemoryStore, wantSkill, wantMemory string) { + t.Helper() + gotSkills := skills.List() + if len(gotSkills) != 1 || gotSkills[0].Name != wantSkill { + t.Fatalf("learned skills = %+v, want %q", gotSkills, wantSkill) + } + gotMemory := memory.List() + if len(gotMemory) != 1 || gotMemory[0].File != wantMemory { + t.Fatalf("learned memory = %+v, want %q", gotMemory, wantMemory) + } +} + +func writeTestFile(t *testing.T, path, content string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} diff --git a/internal/app/services.go b/internal/app/services.go index dadb497f..dddf5574 100644 --- a/internal/app/services.go +++ b/internal/app/services.go @@ -65,6 +65,11 @@ type SelfLearnServices struct { // surface"). Always non-nil; the snapshot reports an idle phase when // L1 is off or no review has run yet. Indicator *SelfLearnIndicator + + // Recent is the rolling activity log behind the /evolve RECENT zones. + // Always non-nil; the per-session write observers append to it and it + // outlives any one session so the recap survives /clear. + Recent *RecentLearnLog } // selfLearnSession bundles the three handles that make up one live L1 @@ -105,6 +110,6 @@ func newServices() services { Agent: agent.Default(), Persona: persona.Default(), Reminder: reminder.NewService(), - SelfLearn: SelfLearnServices{Indicator: NewSelfLearnIndicator()}, + SelfLearn: SelfLearnServices{Indicator: NewSelfLearnIndicator(), Recent: NewRecentLearnLog()}, } } diff --git a/internal/app/update.go b/internal/app/update.go index fc12602d..82321eb0 100644 --- a/internal/app/update.go +++ b/internal/app/update.go @@ -76,6 +76,7 @@ func (m *model) overlayPanels() []overlayPanel { &m.userInput.Search, &m.userInput.Config, &m.userInput.Autopilot, + &m.userInput.Evolve, } } @@ -200,12 +201,13 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.conv.AddNotice("Self-learning config saved (" + msg.Scope + ")") m.notifySelfLearnOverride(msg) - // Re-wire the L1 reviewer so the just-saved arms / cadences take - // effect on the running session instead of silently waiting for - // the next agent restart. Wire only when the agent is already - // active; an inactive session will wire on the first user turn. + // Re-wire the L1 reviewer so the saved values take effect on the + // running session. If the save also changed the Evolve tool's + // capabilities, the toolset is reconciled when the next turn starts — + // ensureAgentSession rebuilds on capability drift (agentEvolveCaps), + // which covers external settings edits the same way. if m.services.Agent.Active() { - m.wireSelfLearn(m.buildAgentParams(), "") + m.wireSelfLearn(m.buildAgentParams()) } return m, nil case input.ThemeSavedMsg: diff --git a/internal/command/registry.go b/internal/command/registry.go index 93549625..b356eec3 100644 --- a/internal/command/registry.go +++ b/internal/command/registry.go @@ -47,7 +47,8 @@ func builtinCommands() []Info { {Name: "think", Description: "Toggle provider-native thinking effort"}, {Name: "loop", Description: "Schedule recurring or one-shot prompts and manage loop jobs"}, {Name: "search", Description: "Select search engine for web search"}, - {Name: "config", Description: "Configure self-learning and other settings"}, + {Name: "config", Description: "Configure appearance and other settings"}, + {Name: "evolve", Description: "Configure self-learning (skills & memory)"}, {Name: "autopilot", Description: "Configure the autopilot copilot (steers, system prompt, mission)"}, {Name: "name", Description: "Set or change the name of the current conversation session"}, {Name: "quit", Description: "Exit the application (/exit also works)"}, diff --git a/internal/core/system/memory.go b/internal/core/system/memory.go index 89d8afc9..414b875b 100644 --- a/internal/core/system/memory.go +++ b/internal/core/system/memory.go @@ -58,14 +58,37 @@ func AutoMemoryIndexPath(cwd string) string { return filepath.Join(AutoMemoryDir(cwd), AutoMemoryIndexName) } -// LoadAutoMemory reads the agent-written auto-memory index for cwd, capped at -// autoMemoryByteCap. It is a distinct source from LoadMemoryFiles: agent-written -// memory and user-authored SAN.md/CLAUDE.md instructions are injected as -// separate blocks and never mixed. Returns ("", false) when the store is empty -// or absent. When the index exceeds the cap it is truncated on a line boundary -// with a marker — topic files are read on demand and never injected. -func LoadAutoMemory(cwd string) (string, bool) { - data, err := os.ReadFile(AutoMemoryIndexPath(cwd)) +// ResolveAutoMemoryDir returns the directory backing the auto-memory store, +// honoring a user override (the /evolve Memory storage path). An empty override +// falls back to the project-partitioned default; a "~" prefix expands to the +// home dir; a relative override resolves against cwd. +func ResolveAutoMemoryDir(cwd, override string) string { + override = strings.TrimSpace(override) + if override == "" { + return AutoMemoryDir(cwd) + } + if override == "~" || strings.HasPrefix(override, "~/") { + if home, err := os.UserHomeDir(); err == nil { + override = filepath.Join(home, strings.TrimPrefix(override, "~")) + } + } + if !filepath.IsAbs(override) { + override = filepath.Join(cwd, override) + } + return override +} + +// LoadAutoMemoryAt reads the agent-written auto-memory index from dir — +// resolve it with ResolveAutoMemoryDir so a user-configured memory path is +// honored (the reviewer prompt and the main-agent memory reminder both do). +// Capped at autoMemoryByteCap. It is a distinct source from LoadMemoryFiles: +// agent-written memory and user-authored SAN.md/CLAUDE.md instructions are +// injected as separate blocks and never mixed. Returns ("", false) when the +// store is empty or absent. When the index exceeds the cap it is truncated on +// a line boundary with a marker — topic files are read on demand and never +// injected. +func LoadAutoMemoryAt(dir string) (string, bool) { + data, err := os.ReadFile(filepath.Join(dir, AutoMemoryIndexName)) if err != nil { return "", false } diff --git a/internal/core/system/memory_test.go b/internal/core/system/memory_test.go index 554853a6..a5ce0935 100644 --- a/internal/core/system/memory_test.go +++ b/internal/core/system/memory_test.go @@ -529,3 +529,25 @@ func TestMemory_MissingFile_NoError(t *testing.T) { _, project := LoadInstructions(tmpDir) _ = project } + +func TestResolveAutoMemoryDir(t *testing.T) { + cwd := "/work/project-x" + // Empty override → the project-partitioned default. + if got := ResolveAutoMemoryDir(cwd, ""); got != AutoMemoryDir(cwd) { + t.Fatalf("empty override: got %q, want default %q", got, AutoMemoryDir(cwd)) + } + // Absolute override is used verbatim. + if got := ResolveAutoMemoryDir(cwd, "/srv/mem"); got != "/srv/mem" { + t.Fatalf("absolute override: got %q", got) + } + // Relative override resolves against cwd. + if got := ResolveAutoMemoryDir(cwd, "notes/mem"); got != filepath.Join(cwd, "notes/mem") { + t.Fatalf("relative override: got %q", got) + } + // "~" expands to the home dir. + if home, err := os.UserHomeDir(); err == nil { + if got := ResolveAutoMemoryDir(cwd, "~/mem"); got != filepath.Join(home, "mem") { + t.Fatalf("tilde override: got %q, want %q", got, filepath.Join(home, "mem")) + } + } +} diff --git a/internal/selflearn/concurrency_test.go b/internal/selflearn/concurrency_test.go index e4ff0f63..8c66d907 100644 --- a/internal/selflearn/concurrency_test.go +++ b/internal/selflearn/concurrency_test.go @@ -21,14 +21,14 @@ import ( func TestSnapshotIsCopiedBeforeGoroutine(t *testing.T) { release := make(chan struct{}) done := make(chan int) - review := func(_ ReviewKind, snapshot []core.Message) { + review := func(_ ReviewKind, _ SkillPermissions, snapshot []core.Message) { <-release done <- len(snapshot) } - r := New(Config{Memory: Arm{Enabled: true, Interval: 1}}, review) + r := New(Config{MemoryEnabled: true}, review) original := []core.Message{{Role: core.RoleUser, Content: "a"}} - r.Observe(core.Result{StopReason: core.StopEndTurn, Messages: original}) + r.Observe(core.Result{StopReason: core.StopEndTurn, Messages: original}, false, true) // Truncate the caller's slice while the goroutine is blocked. If // Observe leaked the slice header the goroutine would later see len 0. @@ -56,7 +56,7 @@ func TestConcurrentObserveIsRaceFree(t *testing.T) { var fired atomic.Int64 hold := make(chan struct{}) fireSignal := make(chan struct{}, 1) - review := func(_ ReviewKind, _ []core.Message) { + review := func(_ ReviewKind, _ SkillPermissions, _ []core.Message) { fired.Add(1) select { case fireSignal <- struct{}{}: // announce the first fire @@ -65,8 +65,8 @@ func TestConcurrentObserveIsRaceFree(t *testing.T) { <-hold // block until the test releases — keeps inFlight=true } r := New(Config{ - Memory: Arm{Enabled: true, Interval: 1}, - Skills: Arm{Enabled: true, Interval: 1}, + MemoryEnabled: true, + Skills: SkillPermissions{AllowCreate: true}, }, review) const goroutines, perG = 8, 20 @@ -76,7 +76,7 @@ func TestConcurrentObserveIsRaceFree(t *testing.T) { go func() { defer wg.Done() for range perG { - r.Observe(endTurn(1)) + r.Observe(endTurn(1), false, true) } }() } diff --git a/internal/selflearn/config.go b/internal/selflearn/config.go index 83d302fa..c941bba6 100644 --- a/internal/selflearn/config.go +++ b/internal/selflearn/config.go @@ -10,15 +10,21 @@ import ( // NewSkillManager, and New. Built once per session via ResolveSettings — // the single bridge between the setting layer and this package. type Config struct { - Memory Arm // memory arm: enable + every-N-turns cadence - Skills Arm // skills arm: enable + every-N-tool-iters cadence - Perms ActionPermissions + MemoryEnabled bool MemoryMaxChars int + MemoryPath string // auto-memory dir override (empty ⇒ project default) + + // Skills bounds what a model-triggered review may do to the skill set. + Skills SkillPermissions + + // Strategy is the user's learning-strategy override; non-empty replaces + // the built-in guidance for both arms in the reviewer prompt. + Strategy string } // Enabled reports whether any arm is on. When false the caller should not // even construct a Reviewer (zero overhead). -func (c Config) Enabled() bool { return c.Memory.Enabled || c.Skills.Enabled } +func (c Config) Enabled() bool { return c.MemoryEnabled || c.Skills.Any() } // ResolveSettings validates the raw settings and returns the resolved // Config, applying §3.1 defaults for unset fields. @@ -27,14 +33,14 @@ func ResolveSettings(s setting.SelfLearnSettings) (Config, error) { return Config{}, fmt.Errorf("self-learning config invalid: %w", err) } return Config{ - Memory: Arm{Enabled: s.Memory.Enabled, Interval: s.Memory.ResolvedEveryTurns()}, - Skills: Arm{Enabled: s.Skills.Enabled, Interval: s.Skills.ResolvedEveryToolIters()}, - Perms: ActionPermissions{ - AllowCreate: s.Skills.AllowCreate(), - AllowUpdate: s.Skills.AllowUpdate(), - AllowDelete: s.Skills.AllowDelete(), - AllowUpdateUserCreated: s.Skills.AllowUpdateUserCreated, - }, + MemoryEnabled: s.Memory.Enabled, MemoryMaxChars: s.Memory.ResolvedMaxKB() * 1024, + MemoryPath: s.Memory.Path, + Skills: SkillPermissions{ + AllowCreate: s.Skills.AllowCreate(), + AllowUpdate: s.Skills.AllowUpdate(), + AllowDelete: s.Skills.AllowDelete(), + }, + Strategy: s.Strategy, }, nil } diff --git a/internal/selflearn/config_test.go b/internal/selflearn/config_test.go index 7140c2d9..f1c167ec 100644 --- a/internal/selflearn/config_test.go +++ b/internal/selflearn/config_test.go @@ -11,30 +11,23 @@ import ( // to a resolved Config and that defaults apply where fields are unset. func TestResolveSettingsHappyPath(t *testing.T) { s := setting.SelfLearnSettings{ - Memory: setting.SelfLearnMemory{Enabled: true, EveryTurns: 7}, // MaxKB unset → default - Skills: setting.SelfLearnSkills{ - Enabled: true, - EveryToolIters: 15, - // Allow* unset → default true - AllowUpdateUserCreated: true, - }, + Memory: setting.SelfLearnMemory{Enabled: true}, // MaxKB unset → default + Skills: setting.SelfLearnSkills{}, // Deny* unset → all three actions allowed + Strategy: "custom strategy", } r, err := ResolveSettings(s) if err != nil { t.Fatalf("happy path failed: %v", err) } - if !r.Memory.Enabled || r.Memory.Interval != 7 { - t.Fatalf("memory arm: %+v", r.Memory) + if !r.MemoryEnabled { + t.Fatalf("memory arm: %+v", r) } - if !r.Skills.Enabled || r.Skills.Interval != 15 { - t.Fatalf("skill arm: %+v", r.Skills) + wantSkills := AllowAllSkillActions() + if r.Skills != wantSkills { + t.Fatalf("skill permissions: got %+v, want %+v", r.Skills, wantSkills) } - wantPerms := ActionPermissions{ - AllowCreate: true, AllowUpdate: true, AllowDelete: true, - AllowUpdateUserCreated: true, - } - if r.Perms != wantPerms { - t.Fatalf("perms: got %+v, want %+v", r.Perms, wantPerms) + if r.Strategy != "custom strategy" { + t.Fatalf("strategy override: got %q", r.Strategy) } if r.MemoryMaxChars != 25*1024 { t.Fatalf("memory cap: got %d, want %d", r.MemoryMaxChars, 25*1024) @@ -51,7 +44,7 @@ func TestResolveSettingsRejectsInvalid(t *testing.T) { if err == nil { t.Fatal("expected validation error to propagate") } - if !strings.Contains(err.Error(), `"Create new skills" needs "Update existing skills"`) { + if !strings.Contains(err.Error(), `"Create new skills" needs "Update a skill"`) { t.Fatalf("error not from Validate: %v", err) } } diff --git a/internal/selflearn/fork.go b/internal/selflearn/fork.go index a3b2aa7f..17f6c51e 100644 --- a/internal/selflearn/fork.go +++ b/internal/selflearn/fork.go @@ -20,13 +20,18 @@ const forkDeadline = 5 * time.Minute // ForkConfig carries everything RunReview needs to fork a restricted reviewer // agent. LLM and System come from the parent so the fork inherits its provider -// and (verbatim) system prompt; Memory and Skills are the write surfaces. +// and (verbatim) system prompt; Memory and Skills are the write surfaces. The +// skill actions this pass may take are read from Skills.Perms() — the prompt is +// tailored to them and the manager enforces them at dispatch. type ForkConfig struct { LLM core.LLM System core.System // parent's system — read for its prompt only CWD string Memory *MemoryStore Skills *SkillManager + // Strategy is the user's optional learning-strategy override; non-empty + // replaces the built-in guidance for both arms. + Strategy string // OnEvent receives the fork agent's lifecycle events (PreInfer / // PostInfer / OnAppend / …). Wire a sidechain recorder here to land // the fork's LLM calls in the main session transcript so the @@ -39,7 +44,7 @@ type ForkConfig struct { // summary, or "nothing to save"). Bounded by forkDeadline. Best-effort: // the caller must run it on a background goroutine. func RunReview(ctx context.Context, fc ForkConfig, kinds ReviewKind, snapshot []core.Message) (string, error) { - prompt := buildReviewPrompt(kinds, fc.CWD, fc.Memory, fc.Skills) + prompt := buildReviewPrompt(kinds, fc.CWD, fc.Memory, fc.Skills, fc.Strategy) ctx, cancel := context.WithTimeout(ctx, forkDeadline) defer cancel() diff --git a/internal/selflearn/fork_test.go b/internal/selflearn/fork_test.go index c2baa747..f333e349 100644 --- a/internal/selflearn/fork_test.go +++ b/internal/selflearn/fork_test.go @@ -83,7 +83,7 @@ func TestTrimTrailingPendingMessages(t *testing.T) { func TestAllowOnlyPolicy(t *testing.T) { store := newTestStore(t) - tools := core.NewTools(newMemoryWriteTool(store), newSkillManageTool(NewSkillManager("/tmp", DefaultActionPermissions()))) + tools := core.NewTools(newMemoryWriteTool(store), newSkillManageTool(NewSkillManager("/tmp", AllowAllSkillActions()))) policy := allowOnly(tools) for _, name := range []string{"memory_write", "skill_manage"} { @@ -101,9 +101,9 @@ func TestAllowOnlyPolicy(t *testing.T) { func TestBuildReviewPromptSelectsArms(t *testing.T) { store := newTestStore(t) mustAdd(t, store, "existing fact about the build") - mgr := NewSkillManager("/work/project-x", DefaultActionPermissions()) + mgr := NewSkillManager("/work/project-x", SkillPermissions{AllowCreate: true, AllowUpdate: true}) - memOnly := buildReviewPrompt(KindMemory, "/work/project-x", store, mgr) + memOnly := buildReviewPrompt(KindMemory, "/work/project-x", store, mgr, "") if !strings.Contains(memOnly, "existing fact about the build") { t.Fatal("memory prompt should embed the current store") } @@ -111,12 +111,12 @@ func TestBuildReviewPromptSelectsArms(t *testing.T) { t.Fatal("memory-only prompt should not include the skill section") } - skillOnly := buildReviewPrompt(KindSkills, "/work/project-x", store, mgr) + skillOnly := buildReviewPrompt(KindSkills, "/work/project-x", store, mgr, "") if !strings.Contains(skillOnly, "skill_manage tool") { t.Fatal("skill prompt should include the skill section") } - combined := buildReviewPrompt(KindMemory|KindSkills, "/work/project-x", store, mgr) + combined := buildReviewPrompt(KindMemory|KindSkills, "/work/project-x", store, mgr, "") if !strings.Contains(combined, "memory_write tool") || !strings.Contains(combined, "skill_manage tool") { t.Fatal("combined prompt should include both sections") } @@ -124,7 +124,7 @@ func TestBuildReviewPromptSelectsArms(t *testing.T) { func TestRunReviewWritesMemoryAndInheritsSystem(t *testing.T) { store := newTestStore(t) - mgr := NewSkillManager("/work/project-x", DefaultActionPermissions()) + mgr := NewSkillManager("/work/project-x", AllowAllSkillActions()) llm := &scriptedLLM{responses: []core.InferResponse{ { @@ -181,7 +181,7 @@ func TestRunReviewWritesMemoryAndInheritsSystem(t *testing.T) { // already pointed at the temp dir by newTestStore). func readBackMemory(t *testing.T) (string, bool) { t.Helper() - store := NewMemoryStore("/work/project-x", 0) + store := NewMemoryStore("/work/project-x", 0, "") entries := readEntries(store.Dir() + "/MEMORY.md") if len(entries) == 0 { return "", false diff --git a/internal/selflearn/memory.go b/internal/selflearn/memory.go index a3e9e642..d557a213 100644 --- a/internal/selflearn/memory.go +++ b/internal/selflearn/memory.go @@ -65,14 +65,15 @@ type MemoryStore struct { mu sync.Mutex } -// NewMemoryStore returns the store for cwd's project partition. maxFile is -// the per-file char cap; pass <= 0 for DefaultMemoryFileCharLimit. The +// NewMemoryStore returns the store for cwd's project partition, or for +// dirOverride when non-empty (the /evolve Memory storage path). maxCharsPerFile +// caps each memory file; pass <= 0 for DefaultMemoryFileCharLimit. The // directory is created lazily on the first write. -func NewMemoryStore(cwd string, maxFile int) *MemoryStore { - if maxFile <= 0 { - maxFile = DefaultMemoryFileCharLimit +func NewMemoryStore(cwd string, maxCharsPerFile int, dirOverride string) *MemoryStore { + if maxCharsPerFile <= 0 { + maxCharsPerFile = DefaultMemoryFileCharLimit } - return &MemoryStore{dir: system.AutoMemoryDir(cwd), maxFile: maxFile} + return &MemoryStore{dir: system.ResolveAutoMemoryDir(cwd, dirOverride), maxFile: maxCharsPerFile} } // MaxKB returns the per-file cap in kilobytes, rounded down. Used by the diff --git a/internal/selflearn/memory_test.go b/internal/selflearn/memory_test.go index 86a69687..6f2b65e0 100644 --- a/internal/selflearn/memory_test.go +++ b/internal/selflearn/memory_test.go @@ -18,7 +18,7 @@ func newTestStore(t *testing.T) *MemoryStore { t.Setenv("HOME", home) t.Setenv("USERPROFILE", home) // Windows cwd := "/work/project-x" - store := NewMemoryStore(cwd, 0) + store := NewMemoryStore(cwd, 0, "") if got := store.Dir(); got != system.AutoMemoryDir(cwd) { t.Fatalf("store dir = %q, want %q", got, system.AutoMemoryDir(cwd)) } @@ -206,13 +206,13 @@ func TestLoadAutoMemoryRoundTrip(t *testing.T) { store := newTestStore(t) cwd := "/work/project-x" - if _, ok := system.LoadAutoMemory(cwd); ok { + if _, ok := system.LoadAutoMemoryAt(system.AutoMemoryDir(cwd)); ok { t.Fatal("empty store should report no auto-memory") } mustAdd(t, store, "first durable fact") - got, ok := system.LoadAutoMemory(cwd) + got, ok := system.LoadAutoMemoryAt(system.AutoMemoryDir(cwd)) if !ok || !strings.Contains(got, "first durable fact") { - t.Fatalf("LoadAutoMemory = %q, ok=%v", got, ok) + t.Fatalf("LoadAutoMemoryAt = %q, ok=%v", got, ok) } } diff --git a/internal/selflearn/prompts.go b/internal/selflearn/prompts.go index 45965c33..b5273c05 100644 --- a/internal/selflearn/prompts.go +++ b/internal/selflearn/prompts.go @@ -13,23 +13,49 @@ import ( // the fork refreshes/dedupes rather than blindly appending. // // Per §5.5 the skill section is synthesised against the SkillManager's -// ActionPermissions: disallowed actions are stripped so the model doesn't -// waste turns proposing them. The hard floor remains the permission gate at +// permissions: disallowed actions are stripped so the model doesn't waste +// turns proposing them. The hard floor remains the permission gate at // dispatch — this is just steering. // // See notes/active/l1-background-review.md §3. -func buildReviewPrompt(kinds ReviewKind, cwd string, memory *MemoryStore, skills *SkillManager) string { +func buildReviewPrompt(kinds ReviewKind, cwd string, memory *MemoryStore, skills *SkillManager, strategy string) string { var b strings.Builder b.WriteString(reviewPreamble) b.WriteString("\n\n") b.WriteString(reviewToolScope) + // Guidance: the user's /evolve Strategy override replaces the built-in + // per-arm sections wholesale; empty ⇒ the tailored built-in for whichever + // arms fired. + b.WriteString("\n\n") + if strategy != "" { + b.WriteString(strategy) + } else { + var sections []string + if kinds.Has(KindMemory) { + sections = append(sections, memorySectionFor(memory)) + } + if kinds.Has(KindSkills) { + perms := AllowAllSkillActions() + if skills != nil { + perms = skills.Perms() + } + sections = append(sections, skillSectionFor(perms)) + } + b.WriteString(strings.Join(sections, "\n\n")) + } + + // Dynamic context is always injected (it is state, not editable guidance). if kinds.Has(KindMemory) { - b.WriteString("\n\n") - b.WriteString(memorySectionFor(memory)) b.WriteString("\n\nCurrent memory store (MEMORY.md):\n") - if mem, ok := system.LoadAutoMemory(cwd); ok { + // Read from the store's own dir so a configured memory path is honored; + // fall back to the cwd default when no store was supplied. + memDir := system.AutoMemoryDir(cwd) + if memory != nil { + memDir = memory.Dir() + } + if mem, ok := system.LoadAutoMemoryAt(memDir); ok { b.WriteString("```\n") b.WriteString(mem) b.WriteString("\n```") @@ -37,10 +63,7 @@ func buildReviewPrompt(kinds ReviewKind, cwd string, memory *MemoryStore, skills b.WriteString("(empty — no entries yet)") } } - if kinds.Has(KindSkills) { - b.WriteString("\n\n") - b.WriteString(skillSectionFor(skills)) b.WriteString("\n\nExisting skills:\n") b.WriteString(renderInventory(skills)) } @@ -50,62 +73,88 @@ func buildReviewPrompt(kinds ReviewKind, cwd string, memory *MemoryStore, skills return b.String() } -// skillSectionFor returns the skill review-prompt section tailored to the -// permissions the SkillManager will enforce at dispatch. See §5.5: stripping -// disallowed actions from the prompt prevents the model from proposing things -// it can't do — the permission layer is still the hard floor. -func skillSectionFor(mgr *SkillManager) string { - perms := DefaultActionPermissions() - if mgr != nil { - perms = mgr.Perms() - } +// DefaultStrategy returns the built-in learning strategy the /evolve Strategy +// editor seeds with: the general memory guidance plus the general skill +// guidance (both the used-a-skill and the new-work cases), so a saved override +// applies to every pass. The live built-in (memorySectionFor / skillSectionFor +// above) is instead tailored per pass to the arms that fired. +func DefaultStrategy() string { + return memorySectionFor(nil) + "\n\n" + + skillPreamble + "\n\n" + + skillCaseUsedHeader + "\n" + skillEvaluateGuidance + "\n\n" + + skillCaseNewHeader + "\n" + skillCreateGuidance + "\n\n" + + skillWriteScope + "\n\n" + skillAntiPatterns +} +// skillSectionFor returns the skill review-prompt section tailored to the +// actions the trigger scoped for this pass: a create pass (no skill was used) +// frames capturing a technique from new work; a skill-use pass frames a single +// integrated decision about the used skill — keep, refine, or retire it. The +// SkillManager's permission gate remains the hard floor. +func skillSectionFor(perms SkillPermissions) string { var b strings.Builder - b.WriteString("SKILLS (skill_manage tool). A skill is a reusable, class-level technique (e.g. go-table-tests), not a session-specific note.") - - // Steer the preference order based on what's actually allowed. + b.WriteString(skillPreamble) + b.WriteString("\n\n") if perms.AllowCreate { - b.WriteString(" Prefer the broadest reuse; create is the last resort.") + b.WriteString(skillCaseNewHeader) + b.WriteString("\n") + b.WriteString(skillCreateGuidance) } else { - b.WriteString(" Creation is disabled: only modify existing skills.") + // A skill was used: whatever actions are allowed, this is ONE decision + // about that skill, never several passes. + b.WriteString(skillCaseUsedHeader) + b.WriteString("\n") + switch { + case perms.AllowUpdate && perms.AllowDelete: + b.WriteString(skillEvaluateGuidance) + case perms.AllowUpdate: + b.WriteString(skillUpdateGuidance) + case perms.AllowDelete: + b.WriteString(skillDeleteGuidance) + } } + b.WriteString("\n\n") + b.WriteString(skillWriteScope) + b.WriteString("\n\n") + b.WriteString(skillAntiPatterns) + return b.String() +} - b.WriteString("\nDecide in this order (preference: UPDATE > DELETE > CREATE):\n") - step := 1 - if perms.AllowUpdate { - fmt.Fprintf(&b, "%d. UPDATE — patch an existing skill when ANY of the following:\n", step) - b.WriteString(" · a skill loaded / consulted this turn was proven wrong, incomplete, or outdated;\n") - b.WriteString(" · an existing umbrella skill covers the new learning (extend it; consider adding a references/templates/scripts support file);\n") - b.WriteString(" · the user voiced a style / format / workflow correction that belongs in the skill governing that task (embed it so the next session starts already knowing).\n") - step++ - } - if perms.AllowDelete { - fmt.Fprintf(&b, "%d. DELETE — retire an agent-created skill when ANY of the following:\n", step) - b.WriteString(" · the new learning supersedes the entire skill wholesale (replace, don't coexist);\n") - b.WriteString(" · the skill encoded a transient / environment-dependent failure that is now resolved (the skill is now wrong);\n") - b.WriteString(" · the skill turned out to encode an anti-pattern.\n") - step++ - } - if perms.AllowCreate { - fmt.Fprintf(&b, "%d. CREATE — only when ALL of the following hold:\n", step) - b.WriteString(" · the turn produced a non-trivial, generalizable technique / fix / pattern;\n") - b.WriteString(" · NO existing skill (agent OR user) covers this class of task;\n") - b.WriteString(" · the name is class-level (e.g. go-table-tests), NOT a PR number, error string, codename, or 'fix-X / debug-Y / audit-Z-today' session artifact;\n") - b.WriteString(" · the learning is not an anti-pattern (see below).\n") - b.WriteString(" Pick the level: reusable / general → user; project-specific → project.\n") - step++ - } +// The skill section is built from first principles: one definition, then the +// two mutually-exclusive cases (a skill ran this turn vs. none did), then the +// hard exclusions. Each case names its trigger, then its decision. - // Scope rule. - if perms.AllowUpdateUserCreated { - b.WriteString("You may patch any existing skill (including user-created); creation and deletion remain restricted to agent-created skills.\n") - } else { - b.WriteString("You may only modify skills marked editable (agent-created); read user-created skills to avoid duplication but never change them.\n") - } +const skillPreamble = `SKILLS — the skill_manage tool writes reusable, class-level techniques (e.g. go-table-tests), never session-specific notes. +Default to changing nothing. Only act on a durable, general learning from THIS turn.` - b.WriteString("ANTI-PATTERNS (do NOT capture as a skill): environment-dependent failures, negative claims about tools, transient errors that resolved on retry, one-off task narratives. If the only candidate falls in this bucket, save nothing.") - return b.String() -} +const skillCaseUsedHeader = `CASE A — a skill ran this turn (find it in the Skill tool calls above). Judge that one skill:` + +const skillCaseNewHeader = `CASE B — no skill ran, but the turn did non-trivial new work worth reusing:` + +// skillEvaluateGuidance frames the both-allowed case: one integrated judgement +// of the used skill's worth, choosing at most one of keep / update / delete. +const skillEvaluateGuidance = ` Pick exactly ONE (improving and retiring are one judgement, not two): + KEEP correct and still useful. The default — prefer it. + UPDATE still useful, but this turn showed it wrong/incomplete/outdated, or the user corrected a style/format/workflow it should carry. Patch the fix in. + DELETE no longer useful: obsolete, superseded, or an anti-pattern. Never delete a skill that still helps.` + +// skillUpdateGuidance is the update-only case (delete disallowed). +const skillUpdateGuidance = ` UPDATE it if this turn showed it wrong/incomplete/outdated, or the user corrected a style/format/workflow it should carry — patch the fix in. Otherwise KEEP it (save nothing).` + +// skillDeleteGuidance is the delete-only case (update disallowed): a pure worth +// check of the used skill. +const skillDeleteGuidance = ` DELETE it only if it no longer helps: obsolete, superseded, wrong, or an anti-pattern. Otherwise KEEP it (save nothing).` + +const skillCreateGuidance = ` CREATE one skill only if ALL hold: + - a non-trivial, generalizable technique/fix/pattern + - no existing skill (yours or the user's — see inventory) already covers it + - a class-level name (go-table-tests), not a PR number, error string, codename, or fix-X/debug-Y + - not an anti-pattern (below) + Scope: general → user; project-specific → project.` + +const skillWriteScope = `Write only your own (agent-created) skills; read the user's to avoid duplication, but never change them.` + +const skillAntiPatterns = `NEVER a skill: environment-specific failures, "tool X is broken" claims, transient errors that passed on retry, one-off task narratives. If that's all you have, save nothing.` func renderInventory(skills *SkillManager) string { if skills == nil { diff --git a/internal/selflearn/prompts_test.go b/internal/selflearn/prompts_test.go index 5f06d595..16387de0 100644 --- a/internal/selflearn/prompts_test.go +++ b/internal/selflearn/prompts_test.go @@ -5,55 +5,37 @@ import ( "testing" ) -// TestSkillSectionAdaptsToPermissions confirms the §5.5 "prompt synthesis" -// rule: actions the SkillManager will veto at dispatch are stripped from the -// review prompt so the model doesn't propose them. -func TestSkillSectionAdaptsToPermissions(t *testing.T) { - t.Run("default — all actions present, conservative scope", func(t *testing.T) { - mgr, _ := newTestSkillManagerWithPerms(t, DefaultActionPermissions()) - s := skillSectionFor(mgr) - mustContain(t, s, "UPDATE — patch") - mustContain(t, s, "DELETE — retire") - mustContain(t, s, "CREATE — only when ALL") - mustContain(t, s, "user voiced a style / format / workflow correction") - mustContain(t, s, "only modify skills marked editable (agent-created)") - mustNotContain(t, s, "Creation is disabled") +// TestSkillSectionIsTriggerAware confirms the review prompt is scoped to the +// actions the trigger allowed this pass: a create pass (no skill used) frames +// capturing novel work; an update/delete pass (a skill was used) frames +// refining/retiring the used skill, and never offers create. +func TestSkillSectionIsTriggerAware(t *testing.T) { + t.Run("create pass — no skill was used", func(t *testing.T) { + s := skillSectionFor(SkillPermissions{AllowCreate: true}) + mustContain(t, s, "CASE B — no skill ran") + mustContain(t, s, "CREATE one skill only if ALL hold") + mustNotContain(t, s, "CASE A") // skill-use framing must not appear }) - t.Run("no create — last-resort line replaced by hard restriction", func(t *testing.T) { - perms := DefaultActionPermissions() - perms.AllowCreate = false - mgr, _ := newTestSkillManagerWithPerms(t, perms) - s := skillSectionFor(mgr) - mustContain(t, s, "Creation is disabled") - mustNotContain(t, s, "CREATE — only when ALL") + t.Run("update+delete pass — one integrated keep/update/delete decision", func(t *testing.T) { + s := skillSectionFor(SkillPermissions{AllowUpdate: true, AllowDelete: true}) + mustContain(t, s, "CASE A — a skill ran this turn") + mustContain(t, s, "Pick exactly ONE") + mustContain(t, s, "Never delete a skill that still helps") + mustNotContain(t, s, "CREATE one skill") }) - t.Run("no update — patch/extend steps removed", func(t *testing.T) { - perms := ActionPermissions{AllowDelete: true} // create/update both off; delete only - mgr, _ := newTestSkillManagerWithPerms(t, perms) - s := skillSectionFor(mgr) - mustNotContain(t, s, "UPDATE — patch") - mustContain(t, s, "DELETE — retire") + t.Run("delete-only pass — pure value assessment", func(t *testing.T) { + s := skillSectionFor(SkillPermissions{AllowDelete: true}) + mustContain(t, s, "DELETE it only if it no longer helps") + mustContain(t, s, "KEEP it (save nothing)") + mustNotContain(t, s, "UPDATE") // the update tool isn't offered this pass }) - t.Run("no delete — retire step removed", func(t *testing.T) { - perms := DefaultActionPermissions() - perms.AllowDelete = false - mgr, _ := newTestSkillManagerWithPerms(t, perms) - s := skillSectionFor(mgr) - mustContain(t, s, "UPDATE — patch") - mustContain(t, s, "CREATE — only when ALL") - mustNotContain(t, s, "DELETE — retire") - }) - - t.Run("advanced opt-in — scope rule widens for patch", func(t *testing.T) { - perms := DefaultActionPermissions() - perms.AllowUpdateUserCreated = true - mgr, _ := newTestSkillManagerWithPerms(t, perms) - s := skillSectionFor(mgr) - mustContain(t, s, "patch any existing skill (including user-created)") - mustNotContain(t, s, "only modify skills marked editable") + t.Run("update-only pass — refine if valuable", func(t *testing.T) { + s := skillSectionFor(SkillPermissions{AllowUpdate: true}) + mustContain(t, s, "UPDATE it if this turn showed") + mustNotContain(t, s, "DELETE it only if") }) } diff --git a/internal/selflearn/reviewer.go b/internal/selflearn/reviewer.go index 9ffc6717..b0e20f48 100644 --- a/internal/selflearn/reviewer.go +++ b/internal/selflearn/reviewer.go @@ -1,9 +1,9 @@ // Package selflearn implements the self-learning loop. Layer 1 (Reviewer -// in this file) is a per-turn background reviewer that, on cadence, forks +// in this file) is a background reviewer that, when the model requests it, forks // a restricted agent to capture durable memory and skill updates. // See notes/active/l1-background-review.md. // -// The trigger core (cadence, StopEndTurn gate, ≤1-in-flight cap) lives +// The trigger core (model request, StopEndTurn gate, ≤1-in-flight cap) lives // here; the fork+review runs through an injected ReviewFunc so the // trigger stays unit-testable without an LLM. package selflearn @@ -18,13 +18,6 @@ import ( "github.com/genai-io/san/internal/log" ) -// Arm configures one review arm. ResolveSettings applies the cadence -// default, so Interval is positive on a Config built the normal way. -type Arm struct { - Enabled bool - Interval int -} - // ReviewKind is a bitmask of the arms that fired on a given turn. type ReviewKind uint8 @@ -53,99 +46,77 @@ func (k ReviewKind) String() string { } // ReviewFunc performs the actual fork+review for the fired arms, given the -// snapshot of the just-completed turn's conversation. It runs on a background -// goroutine and must be best-effort (never panic out / never block the user). -// Injected so trigger logic is unit-testable without an LLM. -type ReviewFunc func(kinds ReviewKind, snapshot []core.Message) +// skill actions this pass may take and the snapshot of the just-completed +// turn's conversation. It runs on a background goroutine and must be +// best-effort (never panic out / never block the user). Injected so trigger +// logic is unit-testable without an LLM. +type ReviewFunc func(kinds ReviewKind, skillPerms SkillPermissions, snapshot []core.Message) -// Reviewer owns the per-session counters and fires reviews on cadence. Safe for +// Reviewer fires a self-learning review when the model requests one. Safe for // concurrent use; Observe is the only entry point. type Reviewer struct { - memEnabled bool - skillEnabled bool - memEvery int - skillEvery int - review ReviewFunc - - mu sync.Mutex - turnsSinceMemory int - itersSinceSkill int - inFlight bool + memoryEnabled bool + skillPerms SkillPermissions + review ReviewFunc + + mu sync.Mutex + inFlight bool } -// New builds a Reviewer from cfg's arm config. review is invoked (on its -// own goroutine) when an arm's threshold is reached. Disabled arms never fire. +// New builds a Reviewer from cfg's arm config. review is invoked on its own +// goroutine when the model requests a review (via the Evolve tool). Disabled +// arms never fire. func New(cfg Config, review ReviewFunc) *Reviewer { return &Reviewer{ - memEnabled: cfg.Memory.Enabled, - skillEnabled: cfg.Skills.Enabled, - memEvery: cfg.Memory.Interval, - skillEvery: cfg.Skills.Interval, - review: review, + memoryEnabled: cfg.MemoryEnabled, + skillPerms: cfg.Skills, + review: review, } } -// SeedTurns hydrates the memory counter on session resume so cadence survives a -// process restart (invariant #8). priorUserTurns is the count of user turns -// already in the resumed history. -func (r *Reviewer) SeedTurns(priorUserTurns int) { - if !r.memEnabled || r.memEvery <= 0 { - return - } - r.mu.Lock() - r.turnsSinceMemory = priorUserTurns % r.memEvery - r.mu.Unlock() -} - -// Observe processes one completed turn. Only cleanly-ended turns count; -// cancelled / interrupted / max-steps turns are skipped (never review work the -// user abandoned). When an arm reaches its threshold it fires a review on a -// background goroutine, at most one in flight per Reviewer — a trigger that -// arrives while a prior review runs is dropped (and the counter NOT reset, so -// it fires again next turn) rather than queued. -func (r *Reviewer) Observe(result core.Result) { - if result.StopReason != core.StopEndTurn { +// Observe processes one completed turn. Triggering is model-decided: a review +// fires only when the main agent called the Evolve tool this turn +// (evolveRequested). It then covers every enabled arm — memory if enabled, and +// skills scoped by skillUsed (a skill-use turn weighs update/delete of that +// skill, a skill-free turn weighs create), bounded by the permission gates. +// +// Only cleanly-ended turns count; cancelled / interrupted / max-steps turns are +// skipped (never review work the user abandoned). At most one review is in +// flight per Reviewer — a request arriving while a prior review runs is dropped +// rather than queued. +func (r *Reviewer) Observe(result core.Result, skillUsed, evolveRequested bool) { + if result.StopReason != core.StopEndTurn || !evolveRequested { return } - r.mu.Lock() - if r.memEnabled { - r.turnsSinceMemory++ - } - if r.skillEnabled { - r.itersSinceSkill += result.ToolUses - // Cap at 2× threshold so a long-running review can't queue up 50 - // refires from the tool calls that stacked during it. - if cap := 2 * r.skillEvery; r.itersSinceSkill > cap { - r.itersSinceSkill = cap - } + // Scope the skills pass by the objective fact of skill use so the reviewer + // prompt stays accurate; the reviewer decides the specific action within it. + var skillPerms SkillPermissions + if skillUsed { + skillPerms.AllowUpdate = r.skillPerms.AllowUpdate + skillPerms.AllowDelete = r.skillPerms.AllowDelete + } else { + skillPerms.AllowCreate = r.skillPerms.AllowCreate } var kinds ReviewKind - if r.memEnabled && r.turnsSinceMemory >= r.memEvery { + if r.memoryEnabled { kinds |= KindMemory } - if r.skillEnabled && r.itersSinceSkill >= r.skillEvery { + if skillPerms.Any() { kinds |= KindSkills } if kinds == 0 { - r.mu.Unlock() return } + + r.mu.Lock() if r.inFlight { - // Drop, don't reset: the threshold stays tripped and fires again on - // the next clean turn once the prior review finishes. r.mu.Unlock() log.Logger().Warn("selflearn: skipping review, a prior review is still running") return } r.inFlight = true - if kinds.Has(KindMemory) { - r.turnsSinceMemory = 0 - } - if kinds.Has(KindSkills) { - r.itersSinceSkill = 0 - } r.mu.Unlock() // Defensive copy: result.Messages aliases the main agent's live slice, @@ -168,7 +139,7 @@ func (r *Reviewer) Observe(result core.Result) { r.mu.Unlock() }() if r.review != nil { - r.review(kinds, snapshot) + r.review(kinds, skillPerms, snapshot) } }() } diff --git a/internal/selflearn/reviewer_test.go b/internal/selflearn/reviewer_test.go index d5c33fe3..86f7b3a9 100644 --- a/internal/selflearn/reviewer_test.go +++ b/internal/selflearn/reviewer_test.go @@ -11,24 +11,37 @@ func endTurn(toolUses int) core.Result { return core.Result{StopReason: core.StopEndTurn, ToolUses: toolUses} } -// waitFire returns the kinds of the next fired review, or fails after timeout. -func waitFire(t *testing.T, fired <-chan ReviewKind) ReviewKind { +type fireRec struct { + kind ReviewKind + skillPerms SkillPermissions +} + +// newTestReviewer wires a Reviewer whose review callback records the fired +// (kind, skill permissions) into a buffered channel. +func newTestReviewer(cfg Config) (*Reviewer, chan fireRec) { + fired := make(chan fireRec, 8) + r := New(cfg, func(k ReviewKind, p SkillPermissions, _ []core.Message) { fired <- fireRec{k, p} }) + return r, fired +} + +// waitFire returns the next fired review, or fails after timeout. +func waitFire(t *testing.T, fired <-chan fireRec) fireRec { t.Helper() select { - case k := <-fired: - return k + case f := <-fired: + return f case <-time.After(time.Second): t.Fatal("expected a review to fire, none did") - return 0 + return fireRec{} } } // assertNoFire fails if a review fires within a short window. -func assertNoFire(t *testing.T, fired <-chan ReviewKind) { +func assertNoFire(t *testing.T, fired <-chan fireRec) { t.Helper() select { - case k := <-fired: - t.Fatalf("expected no review, but %v fired", k) + case f := <-fired: + t.Fatalf("expected no review, but %v fired", f.kind) case <-time.After(80 * time.Millisecond): } } @@ -83,132 +96,132 @@ func TestReviewKindString(t *testing.T) { } } -func TestMemoryFiresOnTurnCadence(t *testing.T) { - fired := make(chan ReviewKind, 4) - r := New(Config{Memory: Arm{Enabled: true, Interval: 3}}, func(k ReviewKind, _ []core.Message) { fired <- k }) +// TestMemoryFiresOnRequest: memory fires only when the model requests a review +// (evolveRequested) and memory is enabled — there is no cadence fallback. +func TestMemoryFiresOnRequest(t *testing.T) { + r, fired := newTestReviewer(Config{MemoryEnabled: true}) - r.Observe(endTurn(0)) - r.Observe(endTurn(0)) - assertNoFire(t, fired) // only 2 turns - r.Observe(endTurn(0)) // 3rd turn → fire - if k := waitFire(t, fired); !k.Has(KindMemory) || k.Has(KindSkills) { - t.Fatalf("want memory-only, got %v", k) + r.Observe(endTurn(0), false, false) // no request + r.Observe(endTurn(0), true, false) // still no request + assertNoFire(t, fired) + r.Observe(endTurn(0), false, true) // requested + if f := waitFire(t, fired); !f.kind.Has(KindMemory) || f.kind.Has(KindSkills) { + t.Fatalf("want memory-only, got %v", f.kind) } } -func TestSkillFiresOnToolIterThreshold(t *testing.T) { - fired := make(chan ReviewKind, 4) - r := New(Config{Skills: Arm{Enabled: true, Interval: 5}}, func(k ReviewKind, _ []core.Message) { fired <- k }) +// TestSkillsScopedByUse: on a requested turn the skills pass is scoped by the +// objective fact of skill use — skill-free → create, skill-use → update+delete. +func TestSkillsScopedByUse(t *testing.T) { + cfg := Config{Skills: AllowAllSkillActions()} + + t.Run("skill-free request scopes to create", func(t *testing.T) { + r, fired := newTestReviewer(cfg) + r.Observe(endTurn(1), false, true) + if f := waitFire(t, fired); f.skillPerms != (SkillPermissions{AllowCreate: true}) { + t.Fatalf("got %+v, want create-only", f.skillPerms) + } + }) + t.Run("skill-use request scopes to update+delete", func(t *testing.T) { + r, fired := newTestReviewer(cfg) + r.Observe(endTurn(1), true, true) + if f := waitFire(t, fired); f.skillPerms != (SkillPermissions{AllowUpdate: true, AllowDelete: true}) { + t.Fatalf("got %+v, want update+delete", f.skillPerms) + } + }) +} - r.Observe(endTurn(2)) - r.Observe(endTurn(2)) - assertNoFire(t, fired) // 4 tool-iters < 5 - r.Observe(endTurn(1)) // 5 total → fire - if k := waitFire(t, fired); !k.Has(KindSkills) || k.Has(KindMemory) { - t.Fatalf("want skill-only, got %v", k) +// TestNoRequestNeverFires: without evolveRequested nothing fires, however much +// work happened — the model's call is the only trigger. +func TestNoRequestNeverFires(t *testing.T) { + r, fired := newTestReviewer(Config{ + MemoryEnabled: true, + Skills: AllowAllSkillActions(), + }) + for range 10 { + r.Observe(endTurn(5), false, false) + r.Observe(endTurn(1), true, false) } + assertNoFire(t, fired) } -func TestCombinedFiresBothArms(t *testing.T) { - fired := make(chan ReviewKind, 4) - r := New(Config{ - Memory: Arm{Enabled: true, Interval: 1}, - Skills: Arm{Enabled: true, Interval: 3}, - }, func(k ReviewKind, _ []core.Message) { fired <- k }) +// TestRequestBoundedByPermissions: a skill-use request with update/delete denied +// yields nothing due; a skill-free request still creates. +func TestRequestBoundedByPermissions(t *testing.T) { + r, fired := newTestReviewer(Config{Skills: SkillPermissions{AllowCreate: true}}) + r.Observe(endTurn(1), true, true) // skill used, update/delete denied → nothing to do + assertNoFire(t, fired) + r.Observe(endTurn(1), false, true) // skill-free → create allowed + if f := waitFire(t, fired); f.skillPerms != (SkillPermissions{AllowCreate: true}) { + t.Fatalf("got %+v, want create-only", f.skillPerms) + } +} - r.Observe(endTurn(3)) // memory due (1 turn) AND skills due (3 iters) - k := waitFire(t, fired) - if !k.Has(KindMemory) || !k.Has(KindSkills) { - t.Fatalf("want combined, got %v", k) +// TestCombinedFiresBothArms: a requested turn with memory on and skills active +// fires both arms. +func TestCombinedFiresBothArms(t *testing.T) { + r, fired := newTestReviewer(Config{ + MemoryEnabled: true, + Skills: SkillPermissions{AllowCreate: true, AllowUpdate: true}, + }) + r.Observe(endTurn(1), false, true) + f := waitFire(t, fired) + if !f.kind.Has(KindMemory) || !f.kind.Has(KindSkills) { + t.Fatalf("want combined, got %v", f.kind) + } + if !f.skillPerms.AllowCreate { + t.Fatalf("skill-free turn should be a create pass, got %+v", f.skillPerms) } } +// TestSkipsNonEndTurn: cancelled / max-steps turns never fire, even when the +// model requested a review. func TestSkipsNonEndTurn(t *testing.T) { - fired := make(chan ReviewKind, 4) - r := New(Config{Memory: Arm{Enabled: true, Interval: 1}}, func(k ReviewKind, _ []core.Message) { fired <- k }) + r, fired := newTestReviewer(Config{MemoryEnabled: true}) - r.Observe(core.Result{StopReason: core.StopCancelled, ToolUses: 9}) - r.Observe(core.Result{StopReason: core.StopMaxSteps}) - assertNoFire(t, fired) // neither counted + r.Observe(core.Result{StopReason: core.StopCancelled}, false, true) + r.Observe(core.Result{StopReason: core.StopMaxSteps}, false, true) + assertNoFire(t, fired) - r.Observe(endTurn(0)) // clean turn → fires - if k := waitFire(t, fired); !k.Has(KindMemory) { - t.Fatalf("want memory, got %v", k) + r.Observe(endTurn(0), false, true) + if f := waitFire(t, fired); !f.kind.Has(KindMemory) { + t.Fatalf("want memory, got %v", f.kind) } } -func TestConcurrencyCapDropsAndRetries(t *testing.T) { - fired := make(chan ReviewKind, 4) +// TestSingleFlightDropsConcurrent: a request arriving while a prior review runs +// is dropped (not queued). Once it clears, a fresh request fires normally. +func TestSingleFlightDropsConcurrent(t *testing.T) { + fired := make(chan fireRec, 4) release := make(chan struct{}) started := make(chan struct{}, 4) - r := New(Config{Memory: Arm{Enabled: true, Interval: 1}}, func(k ReviewKind, _ []core.Message) { + r := New(Config{MemoryEnabled: true}, func(k ReviewKind, p SkillPermissions, _ []core.Message) { started <- struct{}{} <-release // block until released → keeps the review in-flight - fired <- k + fired <- fireRec{k, p} }) - r.Observe(endTurn(0)) // fires review #1 (now in-flight, blocked) + r.Observe(endTurn(0), false, true) // fires review #1 (now in-flight, blocked) mustStart(t, started) - r.Observe(endTurn(0)) // arrives while #1 in-flight → dropped, NOT reset + r.Observe(endTurn(0), false, true) // arrives while #1 in-flight → dropped select { case <-started: t.Fatal("a second review started while one was in flight") case <-time.After(80 * time.Millisecond): } - close(release) // let #1 finish (and any later review won't block now) - if k := waitFire(t, fired); !k.Has(KindMemory) { - t.Fatalf("want memory, got %v", k) + close(release) // let #1 finish + if f := waitFire(t, fired); !f.kind.Has(KindMemory) { + t.Fatalf("want memory, got %v", f.kind) } - // waitFire synced on the fired channel, which the callback sends *before* - // returning; inFlight is only cleared afterwards. Wait for that so the - // retry below fires deterministically instead of racing the reset. waitInFlightClear(t, r) - // The dropped trigger left the counter tripped: the next clean turn fires. - r.Observe(endTurn(0)) + // A fresh request after the prior clears fires normally. + r.Observe(endTurn(0), false, true) mustStart(t, started) - if k := waitFire(t, fired); !k.Has(KindMemory) { - t.Fatalf("retry: want memory, got %v", k) - } -} - -// TestSkillIterCounterCappedDuringInFlight guards against the post-release -// burst: while a long review is in flight every Observe accumulates -// ToolUses indefinitely; without a cap the counter ends up far above the -// threshold and fires on every turn for many turns after the release. -// The cap (2× the threshold) limits this to at most two immediate refires. -func TestSkillIterCounterCappedDuringInFlight(t *testing.T) { - r := New(Config{Skills: Arm{Enabled: true, Interval: 5}}, func(ReviewKind, []core.Message) { - // don't drain — keep inFlight=true on the first trip - }) - // First Observe trips and starts a review that never returns (the - // callback above is a no-op, but inFlight is cleared inside r.run's - // defer — let's instead drive accumulator-only Observes that drop). - r.mu.Lock() - r.inFlight = true // simulate a stuck in-flight review - r.mu.Unlock() - - for range 50 { - r.Observe(endTurn(10)) // 500 cumulative tool-iters across 50 turns - } - r.mu.Lock() - got := r.itersSinceSkill - r.mu.Unlock() - if got > 2*r.skillEvery { - t.Fatalf("itersSinceSkill = %d, expected cap at 2×%d=%d", got, r.skillEvery, 2*r.skillEvery) - } -} - -func TestSeedTurns(t *testing.T) { - fired := make(chan ReviewKind, 4) - r := New(Config{Memory: Arm{Enabled: true, Interval: 5}}, func(k ReviewKind, _ []core.Message) { fired <- k }) - - r.SeedTurns(4) // 4 prior user turns → 1 more should trip the cadence - r.Observe(endTurn(0)) - if k := waitFire(t, fired); !k.Has(KindMemory) { - t.Fatalf("want memory after seed, got %v", k) + if f := waitFire(t, fired); !f.kind.Has(KindMemory) { + t.Fatalf("retry: want memory, got %v", f.kind) } } @@ -216,7 +229,7 @@ func TestConfigEnabled(t *testing.T) { if (Config{}).Enabled() { t.Fatal("empty config should be disabled") } - if !(Config{Skills: Arm{Enabled: true}}).Enabled() { + if !(Config{Skills: SkillPermissions{AllowCreate: true}}).Enabled() { t.Fatal("skills-on should be enabled") } } diff --git a/internal/selflearn/skill.go b/internal/selflearn/skill.go index 2c32d736..a9e3e5ef 100644 --- a/internal/selflearn/skill.go +++ b/internal/selflearn/skill.go @@ -31,21 +31,25 @@ var skillNameRe = regexp.MustCompile(`^[a-z0-9][a-z0-9-]*$`) // supportSubdirs are the only places a skill_manage support file may be written. var supportSubdirs = map[string]struct{}{"references": {}, "templates": {}, "scripts": {}} -// ActionPermissions controls what L1 may do via skill_manage (§5.5). -// The first three flags scope to agent-created skills. AllowUpdateUserCreated -// extends AllowUpdate to user-created skills; create/delete on user-created -// remain impossible at any setting. -type ActionPermissions struct { - AllowCreate bool - AllowUpdate bool - AllowDelete bool - AllowUpdateUserCreated bool +// SkillPermissions controls what L1 may do via skill_manage. All actions +// scope to agent-created skills only — user-authored skills are always +// read-only to the reviewer. It appears at three altitudes with one meaning: +// the configured gates (Config.Skills), the per-pass scope the trigger derives +// from them (a skill-use turn keeps update/delete, a skill-free turn keeps +// create), and the SkillManager's hard floor at dispatch. +type SkillPermissions struct { + AllowCreate bool + AllowUpdate bool + AllowDelete bool } -// DefaultActionPermissions is the safe default: everything allowed within -// the agent-created scope; user-created stays read-only. -func DefaultActionPermissions() ActionPermissions { - return ActionPermissions{AllowCreate: true, AllowUpdate: true, AllowDelete: true} +// Any reports whether at least one action is allowed. +func (p SkillPermissions) Any() bool { return p.AllowCreate || p.AllowUpdate || p.AllowDelete } + +// AllowAllSkillActions is the permissive set: every action allowed within +// the agent-created scope; user-created stays read-only regardless. +func AllowAllSkillActions() SkillPermissions { + return SkillPermissions{AllowCreate: true, AllowUpdate: true, AllowDelete: true} } // SkillWriteObserver fires after every successful write (create/patch/ @@ -61,7 +65,7 @@ type SkillWriteObserver func(action, name, note string) type SkillManager struct { userDir string projectDir string - perms ActionPermissions + perms SkillPermissions onWrite SkillWriteObserver mu sync.Mutex @@ -72,7 +76,7 @@ type SkillManager struct { // the user home directory is unavailable (sandboxed exec, unset HOME), // userDir is left empty and any user-scope write fails loudly via // dirFor — better than silently aliasing user-scope onto cwd/.san/skills. -func NewSkillManager(cwd string, perms ActionPermissions) *SkillManager { +func NewSkillManager(cwd string, perms SkillPermissions) *SkillManager { userDir := "" if home, err := os.UserHomeDir(); err == nil && home != "" { userDir = filepath.Join(confdir.Dir(home), "skills") @@ -85,7 +89,7 @@ func NewSkillManager(cwd string, perms ActionPermissions) *SkillManager { } // Perms returns the current action permissions (read-only snapshot). -func (m *SkillManager) Perms() ActionPermissions { return m.perms } +func (m *SkillManager) Perms() SkillPermissions { return m.perms } // SetWriteObserver registers the callback fired after each successful // write. Must be called before the first write (see type doc). @@ -108,9 +112,8 @@ type SkillInfo struct { Description string } -// Editable reports whether the reviewer may modify this skill at all (agent- -// created). User-created stays read-only at the base level; patching them -// requires the AllowUpdateUserCreated opt-in. +// Editable reports whether the reviewer may modify this skill (agent-created). +// User-created skills are read-only to the reviewer at every setting. func (i SkillInfo) Editable() bool { return i.Origin == agentOrigin } // Inventory lists existing skills across both scopes (project entries shadow @@ -205,6 +208,21 @@ func (m *SkillManager) resolve(name string) (string, error) { return "", fmt.Errorf("no skill named %q", name) } +// Read returns the full SKILL.md content for name, project scope first, for +// read-only display (the /evolve panel's skill preview). resolve enforces the +// same name/traversal guard used by every other on-disk action. +func (m *SkillManager) Read(name string) (string, error) { + path, err := m.resolve(name) + if err != nil { + return "", err + } + b, err := os.ReadFile(path) + if err != nil { + return "", err + } + return string(b), nil +} + // parsed is the result of locating + parsing a skill's SKILL.md once. It // is the return type of the two guards below so callers (Edit, Patch) // don't have to re-parse the same file. @@ -252,26 +270,6 @@ func (m *SkillManager) requireAgentOwned(name string) (parsed, error) { return p, nil } -// requirePatchable parses name and returns it when L1 is allowed to patch -// the body in place. Agent-created skills are always patchable (subject to -// AllowUpdate); anything else (user-created, missing/unrecognised origin) -// is patchable only when AllowUpdateUserCreated is set (§5.5 advanced -// opt-in) — treated as user-created since that is the conservative read -// of any non-agent provenance. -func (m *SkillManager) requirePatchable(name string) (parsed, error) { - p, err := m.parseSkill(name) - if err != nil { - return parsed{}, err - } - if p.origin == agentOrigin || m.perms.AllowUpdateUserCreated { - return p, nil - } - return parsed{}, fmt.Errorf( - "skill %q is user-created; set selfLearn.skills.allowUpdateUserCreated=true to allow patching", - name, - ) -} - func (m *SkillManager) Create(name, description, body, level, note string) (string, error) { if !m.perms.AllowCreate { return "", errActionDenied("create", "allowCreate=false") @@ -336,10 +334,8 @@ func (m *SkillManager) Edit(name, body, note string) (string, error) { m.mu.Lock() defer m.mu.Unlock() - // Edit is a full-body rewrite — restricted to agent-created skills even - // with allowUpdateUserCreated=true. Hermes-style "patch a user skill" is - // targeted; rewriting the whole body of someone's authored file goes too - // far. + // Edit is a full-body rewrite — agent-created only, like every other + // skill_manage action. p, err := m.requireAgentOwned(name) if err != nil { return "", err @@ -359,10 +355,9 @@ func (m *SkillManager) Patch(name, oldText, newText string, replaceAll bool, not m.mu.Lock() defer m.mu.Unlock() - // Patch is the one action whose target scope is extended by - // allowUpdateUserCreated (§5.5). Other update-shaped actions (edit, - // write_file, remove_file, delete) stay agent-created only. - p, err := m.requirePatchable(name) + // Every skill_manage action is agent-created only — the reviewer never + // modifies user-authored skills. + p, err := m.requireAgentOwned(name) if err != nil { return "", err } @@ -470,7 +465,7 @@ func (m *SkillManager) Delete(name, note string) (string, error) { } // errActionDenied builds a uniform "permission denied" error for actions the -// configured ActionPermissions reject. Used as the early-return in the four +// configured SkillPermissions reject. Used as the early-return in the four // action entry points so the model sees a consistent shape on the // permission-veto path (§5.5). func errActionDenied(action, reason string) error { diff --git a/internal/selflearn/skill_test.go b/internal/selflearn/skill_test.go index 9dd40dc4..7f0379e5 100644 --- a/internal/selflearn/skill_test.go +++ b/internal/selflearn/skill_test.go @@ -56,15 +56,15 @@ func TestApplyPatchEscapeDrift(t *testing.T) { } // newTestSkillManager points user/project skill dirs at temp dirs, with -// DefaultActionPermissions (all three flags on, user-created untouched). +// full permissions (all three actions allowed, user-created untouched). func newTestSkillManager(t *testing.T) (*SkillManager, string) { t.Helper() - return newTestSkillManagerWithPerms(t, DefaultActionPermissions()) + return newTestSkillManagerWithPerms(t, AllowAllSkillActions()) } // newTestSkillManagerWithPerms is like newTestSkillManager but lets a test // override the action permission set under exercise. -func newTestSkillManagerWithPerms(t *testing.T, perms ActionPermissions) (*SkillManager, string) { +func newTestSkillManagerWithPerms(t *testing.T, perms SkillPermissions) (*SkillManager, string) { t.Helper() home := t.TempDir() t.Setenv("HOME", home) @@ -251,13 +251,13 @@ func TestSkillRejectsInjectionContent(t *testing.T) { } } -// TestSkillActionPermissions covers the §5.5 boolean matrix: each `allow*` +// TestSkillPermissionsMatrix covers the §5.5 boolean matrix: each `allow*` // flag, when off, must veto its action at dispatch with a uniform // "permission denied" error and leave the on-disk state untouched. -func TestSkillActionPermissions(t *testing.T) { +func TestSkillPermissionsMatrix(t *testing.T) { // allowCreate=false vetoes Create. t.Run("create denied", func(t *testing.T) { - perms := DefaultActionPermissions() + perms := AllowAllSkillActions() perms.AllowCreate = false mgr, _ := newTestSkillManagerWithPerms(t, perms) _, err := mgr.Create("blocked", "x", "Body.", "user", "") @@ -270,13 +270,13 @@ func TestSkillActionPermissions(t *testing.T) { t.Run("update denied across actions", func(t *testing.T) { // Seed a skill with a permissive manager, then exercise a denying // manager pointed at the same disk. - seedPerms := DefaultActionPermissions() + seedPerms := AllowAllSkillActions() seedMgr, cwd := newTestSkillManagerWithPerms(t, seedPerms) if _, err := seedMgr.Create("seeded", "x", "Original body.", "user", ""); err != nil { t.Fatalf("seed: %v", err) } - denyPerms := DefaultActionPermissions() + denyPerms := AllowAllSkillActions() denyPerms.AllowUpdate = false denyMgr := NewSkillManager(cwd, denyPerms) for _, c := range []struct { @@ -302,11 +302,11 @@ func TestSkillActionPermissions(t *testing.T) { // allowDelete=false vetoes Delete. t.Run("delete denied", func(t *testing.T) { - seedMgr, cwd := newTestSkillManagerWithPerms(t, DefaultActionPermissions()) + seedMgr, cwd := newTestSkillManagerWithPerms(t, AllowAllSkillActions()) if _, err := seedMgr.Create("doomed", "x", "Body.", "user", ""); err != nil { t.Fatalf("seed: %v", err) } - denyPerms := DefaultActionPermissions() + denyPerms := AllowAllSkillActions() denyPerms.AllowDelete = false denyMgr := NewSkillManager(cwd, denyPerms) if _, err := denyMgr.Delete("doomed", ""); err == nil || !strings.Contains(err.Error(), "allowDelete=false") { @@ -318,11 +318,12 @@ func TestSkillActionPermissions(t *testing.T) { }) } -// TestSkillAllowUpdateUserCreated covers the single advanced opt-in: it -// extends Patch to user-created skills, but never Edit, Delete, or Create. -func TestSkillAllowUpdateUserCreated(t *testing.T) { +// TestUserCreatedSkillsAreReadOnly confirms the reviewer never modifies a +// user-authored skill: patch, edit, and delete are all refused even at full +// permissions. User-created skills are read-only to L1 at every setting. +func TestUserCreatedSkillsAreReadOnly(t *testing.T) { // Hand-write a user-created skill (no origin field — defaults to user). - mgr, cwd := newTestSkillManagerWithPerms(t, DefaultActionPermissions()) + mgr, _ := newTestSkillManagerWithPerms(t, AllowAllSkillActions()) userDir := filepath.Join(mgr.userDir, "human-authored") if err := os.MkdirAll(userDir, 0o755); err != nil { t.Fatal(err) @@ -332,30 +333,14 @@ func TestSkillAllowUpdateUserCreated(t *testing.T) { t.Fatal(err) } - // Default perms (advanced opt-in OFF): Patch user-created is refused. if _, err := mgr.Patch("human-authored", "Original", "Hacked", false, ""); err == nil { - t.Fatal("default perms must refuse patch on user-created") + t.Fatal("patch on user-created must be refused") } - - // Advanced opt-in ON: Patch goes through. - perms := DefaultActionPermissions() - perms.AllowUpdateUserCreated = true - advMgr := NewSkillManager(cwd, perms) - if _, err := advMgr.Patch("human-authored", "Original user body.", "Refined user body.", false, ""); err != nil { - t.Fatalf("advanced opt-in should allow patch on user-created: %v", err) - } - - // But Edit (full rewrite), Delete, and WriteFile remain forbidden even - // with the advanced opt-in — design invariant: only patch crosses the - // user-created boundary. - if _, err := advMgr.Edit("human-authored", "Wholesale rewrite.", ""); err == nil { - t.Fatal("Edit on user-created must remain forbidden even with allowUpdateUserCreated=true") - } - if _, err := advMgr.Delete("human-authored", ""); err == nil { - t.Fatal("Delete on user-created must remain forbidden at any setting") + if _, err := mgr.Edit("human-authored", "Wholesale rewrite.", ""); err == nil { + t.Fatal("edit on user-created must be refused") } - if _, err := advMgr.WriteFile("human-authored", "references/x.md", "note", ""); err == nil { - t.Fatal("WriteFile on user-created must remain forbidden even with allowUpdateUserCreated=true") + if _, err := mgr.Delete("human-authored", ""); err == nil { + t.Fatal("delete on user-created must be refused") } } diff --git a/internal/setting/clone_test.go b/internal/setting/clone_test.go index 654267c6..abc2fc20 100644 --- a/internal/setting/clone_test.go +++ b/internal/setting/clone_test.go @@ -15,8 +15,9 @@ func TestClonePreservesAllScalarFields(t *testing.T) { AllowBypass: &yes, Persona: "ml-researcher", SelfLearn: SelfLearnSettings{ - Memory: SelfLearnMemory{Enabled: true, EveryTurns: 7, MaxKB: 15}, - Skills: SelfLearnSkills{Enabled: true, DenyCreate: true, AllowUpdateUserCreated: true}, + Memory: SelfLearnMemory{Enabled: true, MaxKB: 15}, + Skills: SelfLearnSkills{DenyCreate: true}, + Strategy: "custom", }, } @@ -51,24 +52,25 @@ func TestClonePreservesAllScalarFields(t *testing.T) { func TestMergeSettingsPreservesSelfLearn(t *testing.T) { base := &Data{ SelfLearn: SelfLearnSettings{ - Memory: SelfLearnMemory{Enabled: true, EveryTurns: 3, MaxKB: 15}, - Skills: SelfLearnSkills{Enabled: false}, + Memory: SelfLearnMemory{Enabled: true, MaxKB: 15}, + Skills: SelfLearnSkills{DenyUpdate: true}, }, } overlay := &Data{ SelfLearn: SelfLearnSettings{ - Skills: SelfLearnSkills{Enabled: true, DenyCreate: true, EveryToolIters: 7}, + Skills: SelfLearnSkills{DenyCreate: true}, + Strategy: "overlay strategy", }, } got := mergeSettings(base, overlay) // Memory comes entirely from base since overlay didn't touch it. - if !got.SelfLearn.Memory.Enabled || got.SelfLearn.Memory.EveryTurns != 3 || got.SelfLearn.Memory.MaxKB != 15 { + if !got.SelfLearn.Memory.Enabled || got.SelfLearn.Memory.MaxKB != 15 { t.Errorf("Memory: got %+v, want base passthrough", got.SelfLearn.Memory) } - // Skills field-merges: overlay's Enabled+DenyCreate+EveryToolIters - // reach the result; base's missing fields stay zero. - if !got.SelfLearn.Skills.Enabled || !got.SelfLearn.Skills.DenyCreate || got.SelfLearn.Skills.EveryToolIters != 7 { + // Skills field-merges: Deny* OR across levels (overlay's DenyCreate + base's + // DenyUpdate both survive); the shared Strategy coalesces from the overlay. + if !got.SelfLearn.Skills.DenyCreate || !got.SelfLearn.Skills.DenyUpdate || got.SelfLearn.Strategy != "overlay strategy" { t.Errorf("Skills: got %+v, want merged overlay", got.SelfLearn.Skills) } diff --git a/internal/setting/merger.go b/internal/setting/merger.go index 814a0490..2aafbfcf 100644 --- a/internal/setting/merger.go +++ b/internal/setting/merger.go @@ -78,18 +78,16 @@ func ApplyPersonaOverlay(base, overlay *Data) *Data { func mergeSelfLearn(base, overlay SelfLearnSettings) SelfLearnSettings { return SelfLearnSettings{ Memory: SelfLearnMemory{ - Enabled: overlay.Memory.Enabled || base.Memory.Enabled, - EveryTurns: coalesceInt(overlay.Memory.EveryTurns, base.Memory.EveryTurns), - MaxKB: coalesceInt(overlay.Memory.MaxKB, base.Memory.MaxKB), + Enabled: overlay.Memory.Enabled || base.Memory.Enabled, + MaxKB: coalesceInt(overlay.Memory.MaxKB, base.Memory.MaxKB), + Path: coalesce(overlay.Memory.Path, base.Memory.Path), }, Skills: SelfLearnSkills{ - Enabled: overlay.Skills.Enabled || base.Skills.Enabled, - EveryToolIters: coalesceInt(overlay.Skills.EveryToolIters, base.Skills.EveryToolIters), - DenyCreate: overlay.Skills.DenyCreate || base.Skills.DenyCreate, - DenyUpdate: overlay.Skills.DenyUpdate || base.Skills.DenyUpdate, - DenyDelete: overlay.Skills.DenyDelete || base.Skills.DenyDelete, - AllowUpdateUserCreated: overlay.Skills.AllowUpdateUserCreated || base.Skills.AllowUpdateUserCreated, + DenyCreate: overlay.Skills.DenyCreate || base.Skills.DenyCreate, + DenyUpdate: overlay.Skills.DenyUpdate || base.Skills.DenyUpdate, + DenyDelete: overlay.Skills.DenyDelete || base.Skills.DenyDelete, }, + Strategy: coalesce(overlay.Strategy, base.Strategy), } } diff --git a/internal/setting/selflearn_test.go b/internal/setting/selflearn_test.go index 836e21cc..0d87f684 100644 --- a/internal/setting/selflearn_test.go +++ b/internal/setting/selflearn_test.go @@ -1,6 +1,9 @@ package setting import ( + "encoding/json" + "os" + "path/filepath" "strings" "testing" ) @@ -23,22 +26,21 @@ func TestSelfLearnValidate(t *testing.T) { { name: "explicit defaults", cfg: SelfLearnSettings{ - Memory: SelfLearnMemory{Enabled: true, EveryTurns: 10, MaxKB: 25}, - Skills: SelfLearnSkills{Enabled: true, EveryToolIters: 10}, + Memory: SelfLearnMemory{Enabled: true, MaxKB: 25}, }, wantErr: "", }, { name: "freeze mode (deny create; update + delete default-allow)", cfg: SelfLearnSettings{ - Skills: SelfLearnSkills{Enabled: true, DenyCreate: true}, + Skills: SelfLearnSkills{DenyCreate: true}, }, wantErr: "", }, { name: "patch-only mode (deny create + delete)", cfg: SelfLearnSettings{ - Skills: SelfLearnSkills{Enabled: true, DenyCreate: true, DenyDelete: true}, + Skills: SelfLearnSkills{DenyCreate: true, DenyDelete: true}, }, wantErr: "", }, @@ -47,26 +49,15 @@ func TestSelfLearnValidate(t *testing.T) { cfg: SelfLearnSettings{ Skills: SelfLearnSkills{DenyUpdate: true}, }, - wantErr: `"Create new skills" needs "Update existing skills"`, + wantErr: `"Create new skills" needs "Update a skill"`, }, { name: "ok: create + update allowed but delete denied (no-auto-delete mode)", cfg: SelfLearnSettings{ - Skills: SelfLearnSkills{Enabled: true, DenyDelete: true}, + Skills: SelfLearnSkills{DenyDelete: true}, }, wantErr: "", }, - { - name: "rejected: advanced opt-in without update base", - cfg: SelfLearnSettings{ - Skills: SelfLearnSkills{ - DenyUpdate: true, - DenyCreate: true, - AllowUpdateUserCreated: true, - }, - }, - wantErr: `"Update user-authored skills" needs "Update existing skills"`, - }, { name: "rejected: maxKB above injection cap", cfg: SelfLearnSettings{ @@ -118,6 +109,69 @@ func TestSelfLearnAllowAccessors(t *testing.T) { } } +func TestSelfLearnSkillsMigratesLegacyDisabledJSON(t *testing.T) { + for _, raw := range []string{ + `{}`, + `{"enabled":false}`, + `{"denyDelete":true}`, + } { + var got SelfLearnSkills + if err := json.Unmarshal([]byte(raw), &got); err != nil { + t.Fatalf("unmarshal %s: %v", raw, err) + } + if got.Active() { + t.Errorf("legacy disabled config %s became active: %+v", raw, got) + } + } +} + +func TestLoaderPreservesLegacySkillOptOut(t *testing.T) { + userDir := t.TempDir() + projectDir := t.TempDir() + path := filepath.Join(userDir, "settings.json") + if err := os.WriteFile(path, []byte(`{"selfLearn":{"skills":{}}}`), 0o644); err != nil { + t.Fatal(err) + } + + got, err := NewLoaderWithOptions(userDir, projectDir, false).Load() + if err != nil { + t.Fatal(err) + } + if got.SelfLearn.Skills.Active() { + t.Fatalf("legacy loader path re-enabled opted-out skills: %+v", got.SelfLearn.Skills) + } +} + +func TestSelfLearnSkillsJSONRoundTripMarksCurrentSchema(t *testing.T) { + want := SelfLearnSkills{DenyDelete: true} + data, err := json.Marshal(want) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(data), `"enabled":true`) { + t.Fatalf("current schema marker missing from %s", data) + } + + var got SelfLearnSkills + if err := json.Unmarshal(data, &got); err != nil { + t.Fatal(err) + } + if got != want { + t.Fatalf("round trip = %+v, want %+v", got, want) + } +} + +func TestSelfLearnSkillsMigratesLegacyEnabledJSON(t *testing.T) { + var got SelfLearnSkills + if err := json.Unmarshal([]byte(`{"enabled":true,"denyDelete":true}`), &got); err != nil { + t.Fatal(err) + } + want := SelfLearnSkills{DenyDelete: true} + if got != want { + t.Fatalf("legacy enabled config = %+v, want %+v", got, want) + } +} + // TestSelfLearnMemoryResolvedMaxKB confirms the default fallback for MaxKB when // unset (zero) and identity for non-zero values. func TestSelfLearnMemoryResolvedMaxKB(t *testing.T) { diff --git a/internal/setting/service.go b/internal/setting/service.go index 4b2cb5bd..5f83696a 100644 --- a/internal/setting/service.go +++ b/internal/setting/service.go @@ -153,6 +153,20 @@ func (s *Settings) StreamIdleTimeout() string { return s.data.StreamIdleTimeout } +// SelfLearn returns just the self-learning settings, by value. Unlike +// Snapshot() it does not deep-clone the whole Data — SelfLearnSettings is a +// value type (no shared maps/pointers), so a plain copy under the read lock is +// safe. Used by the per-turn capability-drift check, which would otherwise +// clone every permission map and hook to read this one field. +func (s *Settings) SelfLearn() SelfLearnSettings { + s.mu.RLock() + defer s.mu.RUnlock() + if s.data == nil { + return SelfLearnSettings{} + } + return s.data.SelfLearn +} + func (s *Settings) Hooks() map[string][]Hook { s.mu.RLock() defer s.mu.RUnlock() diff --git a/internal/setting/settings.go b/internal/setting/settings.go index 832cc558..a9774835 100644 --- a/internal/setting/settings.go +++ b/internal/setting/settings.go @@ -167,11 +167,17 @@ func (a AutoPilotSettings) Equal(b AutoPilotSettings) bool { a.Steers.TurnEnd == b.Steers.TurnEnd } -// SelfLearnSettings configures the two independent self-learning arms. -// See notes/active/l1-background-review.md §3.1. +// SelfLearnSettings configures self-learning. Both arms are triggered together +// by the model (the Evolve tool); the per-arm fields are permission / store +// settings only. See notes/active/l1-background-review.md §3.1. type SelfLearnSettings struct { Memory SelfLearnMemory `json:"memory,omitempty"` Skills SelfLearnSkills `json:"skills,omitempty"` + + // Strategy, when non-empty, replaces the built-in learning strategy in the + // reviewer prompt (edited via the /evolve Strategy entry) — it steers both + // the memory and skill sections. Empty ⇒ built-in. + Strategy string `json:"strategy,omitempty"` } // SelfLearnMaxMemoryKB is the upper bound on memory.maxKB and the default @@ -180,48 +186,84 @@ type SelfLearnSettings struct { // what the loader would have to truncate — see §4.2 invariant. const SelfLearnMaxMemoryKB = 25 -// SelfLearnDefaultEveryTurns and SelfLearnDefaultEveryToolIters are the -// review-cadence defaults applied when the corresponding config field is -// zero — the single source of truth shared by the runtime reviewer -// (ResolveSettings) and the /config panel's default display. -const ( - SelfLearnDefaultEveryTurns = 10 - SelfLearnDefaultEveryToolIters = 10 -) - -// SelfLearnMemory controls memory-evolving: review every N user turns. MaxKB -// is the on-disk cap per memory file; lower values force more aggressive -// pruning. May not exceed SelfLearnMaxMemoryKB. +// SelfLearnMemory controls memory-evolving. MaxKB is the on-disk cap per memory +// file; lower values force more aggressive pruning. May not exceed +// SelfLearnMaxMemoryKB. type SelfLearnMemory struct { - Enabled bool `json:"enabled,omitempty"` - EveryTurns int `json:"everyTurns,omitempty"` // 0 = SelfLearnDefaultEveryTurns - MaxKB int `json:"maxKB,omitempty"` // 0 = SelfLearnMaxMemoryKB + Enabled bool `json:"enabled,omitempty"` + MaxKB int `json:"maxKB,omitempty"` // 0 = SelfLearnMaxMemoryKB + + // Path, when non-empty, overrides where the auto-memory store lives (the + // /evolve Memory storage-path editor). "~" expands to home; a relative path + // resolves against cwd. Empty ⇒ the project-partitioned default. + Path string `json:"path,omitempty"` } -// SelfLearnSkills controls skill-evolving: review when accumulated tool -// iterations since the last review reach EveryToolIters, plus the action -// permissions of §5.5. +// SelfLearnSkills controls skill-evolving. Triggering is model-decided (the +// Evolve tool); these fields only bound what a triggered review may do. The +// review is scoped by skill use: a turn that used a skill weighs UPDATE / +// DELETE of that skill, a skill-free turn weighs CREATE. // -// Permission fields are encoded as Deny* booleans so the zero value is -// "allow" — the Go idiom of "zero value should be sensible default" — and -// every settings.json that omits the field gets the conservative -// permissive default without any pointer-vs-nil ceremony. +// There is no separate on/off toggle: skill-evolving is Active when at least +// one action is allowed. Action gates are encoded as Deny* booleans so the +// zero value is "allow" — the Go idiom of "zero value should be a sensible +// default" — and an omitted field gets the permissive default. (The JSON +// codecs below still read/write an explicit `enabled` marker for +// compatibility with the pre-permission schema.) type SelfLearnSkills struct { - Enabled bool `json:"enabled,omitempty"` - EveryToolIters int `json:"everyToolIters,omitempty"` // 0 = SelfLearnDefaultEveryToolIters - // DenyCreate / DenyUpdate / DenyDelete gate the corresponding action on - // agent-created skills. Zero ⇒ allowed; set to true to disable that - // action. See §5.5. + // agent-created skills. Zero ⇒ allowed; set to true to disable that action. + // They bound what a model-triggered review may do to the skill set. DenyCreate bool `json:"denyCreate,omitempty"` DenyUpdate bool `json:"denyUpdate,omitempty"` DenyDelete bool `json:"denyDelete,omitempty"` +} - // AllowUpdateUserCreated is the advanced opt-in that extends update to - // also patch user-created skills (Hermes-style). Default false. Even - // when true, create and delete on user-created remain impossible at - // any config setting. - AllowUpdateUserCreated bool `json:"allowUpdateUserCreated,omitempty"` +// wireSkills is the JSON shape of SelfLearnSkills: the Deny* gates plus an +// explicit `enabled` marker retained for compatibility with the legacy +// explicit-toggle schema (see the codec methods below). +type wireSkills struct { + Enabled bool `json:"enabled"` + DenyCreate bool `json:"denyCreate,omitempty"` + DenyUpdate bool `json:"denyUpdate,omitempty"` + DenyDelete bool `json:"denyDelete,omitempty"` +} + +// MarshalJSON always writes the `enabled` marker. Before skill-evolving became +// permission-driven, enabled:false was omitted by encoding/json and therefore +// commonly persisted as an empty skills object; the explicit marker lets +// UnmarshalJSON distinguish new permissive configurations from those legacy +// opt-outs. +func (s SelfLearnSkills) MarshalJSON() ([]byte, error) { + return json.Marshal(wireSkills{ + Enabled: s.Active(), + DenyCreate: s.DenyCreate, + DenyUpdate: s.DenyUpdate, + DenyDelete: s.DenyDelete, + }) +} + +// UnmarshalJSON migrates the legacy explicit-toggle schema: a missing or false +// `enabled` (both decode to false) is the old disabled state, so all +// autonomous actions are denied. Files emitted by MarshalJSON always carry +// enabled:true when any action is allowed, so the permission gates round-trip +// exactly. +func (s *SelfLearnSkills) UnmarshalJSON(data []byte) error { + var wire wireSkills + if err := json.Unmarshal(data, &wire); err != nil { + return err + } + *s = SelfLearnSkills{ + DenyCreate: wire.DenyCreate, + DenyUpdate: wire.DenyUpdate, + DenyDelete: wire.DenyDelete, + } + if !wire.Enabled { + s.DenyCreate = true + s.DenyUpdate = true + s.DenyDelete = true + } + return nil } // AllowCreate / AllowUpdate / AllowDelete report whether the named action @@ -231,6 +273,12 @@ func (s SelfLearnSkills) AllowCreate() bool { return !s.DenyCreate } func (s SelfLearnSkills) AllowUpdate() bool { return !s.DenyUpdate } func (s SelfLearnSkills) AllowDelete() bool { return !s.DenyDelete } +// Active reports whether skill-evolving is on — i.e. at least one action is +// allowed. This replaces the old explicit Enabled toggle (implicit enable). +func (s SelfLearnSkills) Active() bool { + return s.AllowCreate() || s.AllowUpdate() || s.AllowDelete() +} + // ResolvedMaxKB returns the resolved MaxKB (default SelfLearnMaxMemoryKB if zero). func (m SelfLearnMemory) ResolvedMaxKB() int { if m.MaxKB <= 0 { @@ -239,24 +287,6 @@ func (m SelfLearnMemory) ResolvedMaxKB() int { return m.MaxKB } -// ResolvedEveryTurns returns the resolved memory review cadence in user turns -// (default SelfLearnDefaultEveryTurns when the field is zero). -func (m SelfLearnMemory) ResolvedEveryTurns() int { - if m.EveryTurns <= 0 { - return SelfLearnDefaultEveryTurns - } - return m.EveryTurns -} - -// ResolvedEveryToolIters returns the resolved skill review cadence in tool -// iterations (default SelfLearnDefaultEveryToolIters when the field is zero). -func (s SelfLearnSkills) ResolvedEveryToolIters() int { - if s.EveryToolIters <= 0 { - return SelfLearnDefaultEveryToolIters - } - return s.EveryToolIters -} - // Validate enforces the cross-field invariants of §3.1: two illegal skill // boolean combinations are rejected, and memory.maxKB must lie in [0, 25]. // Returns nil when the configuration is acceptable (including the all-zero @@ -273,16 +303,9 @@ func (s SelfLearnSettings) Validate() error { SelfLearnMaxMemoryKB, s.Memory.MaxKB, ) } - create := s.Skills.AllowCreate() - update := s.Skills.AllowUpdate() - if create && !update { - return fmt.Errorf( - "\"Create new skills\" needs \"Update existing skills\" — otherwise created skills could never be refined", - ) - } - if s.Skills.AllowUpdateUserCreated && !update { + if s.Skills.AllowCreate() && !s.Skills.AllowUpdate() { return fmt.Errorf( - "\"Update user-authored skills\" needs \"Update existing skills\" — the base update permission", + "\"Create new skills\" needs \"Update a skill\" — otherwise created skills could never be refined", ) } return nil diff --git a/internal/setting/update_selflearn_test.go b/internal/setting/update_selflearn_test.go index 391ab84c..b91a8a4b 100644 --- a/internal/setting/update_selflearn_test.go +++ b/internal/setting/update_selflearn_test.go @@ -38,22 +38,22 @@ func TestUpdateSelfLearnAtPersistsDisable(t *testing.T) { } if err := UpdateSelfLearnAt(SelfLearnSettings{ - Memory: SelfLearnMemory{Enabled: true, EveryTurns: 7}, - Skills: SelfLearnSkills{Enabled: true}, + Memory: SelfLearnMemory{Enabled: true}, + Skills: SelfLearnSkills{}, // all actions allowed → skills active }, true); err != nil { t.Fatalf("enable: %v", err) } - if sl := read(); !sl.Memory.Enabled || !sl.Skills.Enabled { + if sl := read(); !sl.Memory.Enabled || !sl.Skills.Active() { t.Fatalf("after enable, want both on, got %+v", sl) } if err := UpdateSelfLearnAt(SelfLearnSettings{ - Memory: SelfLearnMemory{Enabled: false, EveryTurns: 7}, - Skills: SelfLearnSkills{Enabled: false}, + Memory: SelfLearnMemory{Enabled: false}, + Skills: SelfLearnSkills{DenyCreate: true, DenyUpdate: true, DenyDelete: true}, // all denied → off }, true); err != nil { t.Fatalf("disable: %v", err) } - if sl := read(); sl.Memory.Enabled || sl.Skills.Enabled { + if sl := read(); sl.Memory.Enabled || sl.Skills.Active() { t.Fatalf("disable was dropped (OR-merge regression): %+v", sl) } } diff --git a/internal/tool/evolve/evolve.go b/internal/tool/evolve/evolve.go new file mode 100644 index 00000000..e0bdf98a --- /dev/null +++ b/internal/tool/evolve/evolve.go @@ -0,0 +1,121 @@ +// Package evolve provides the Evolve tool: a lightweight signal the main agent +// calls to request a self-learning review (memory and/or skills) of the work it +// just completed. +// +// The tool performs no learning itself. Its call is observed by the app, which +// fires the L1 background reviewer at turn end (see the self-learning wiring in +// internal/app). It is injected into the main agent's toolset only when +// self-learning is active — with a description tailored to the enabled +// capabilities; otherwise the tool is absent. +package evolve + +import ( + "context" + "strings" + + "github.com/genai-io/san/internal/core" + "github.com/genai-io/san/internal/tool" + "github.com/genai-io/san/internal/tool/toolresult" +) + +const IconEvolve = "✦" + +// Capabilities describes what an accepted self-learning review could persist. +// The tool's schema is built from it so the model is only told about what the +// review can actually save — a turn where memory is off, say, never invites +// the model to flag a memory-only learning it can't act on. +type Capabilities struct { + CreateSkills bool // the review may create new skills + UpdateSkills bool // the review may refine existing skills + DeleteSkills bool // the review may retire existing skills + WriteMemory bool // the review may write durable memory +} + +// Active reports whether any capability is on (⇒ the tool should be present). +func (c Capabilities) Active() bool { + return c.CreateSkills || c.UpdateSkills || c.DeleteSkills || c.WriteMemory +} + +// Schema builds the Evolve tool schema advertised to the main agent, tailored +// to the enabled capabilities so the model is only invited to flag learnings +// the review can act on. The description is deliberately conservative — the +// failure mode to avoid is over-calling on routine turns. The app injects it +// via the toolset's ExtraTools hook only when caps.Active(). +func Schema(caps Capabilities) core.ToolSchema { + var reasons []string + if caps.WriteMemory { + reasons = append(reasons, "- a durable fact worth remembering (a user preference or correction, a project convention, an environment/build/debug insight)") + } + if caps.CreateSkills { + reasons = append(reasons, "- a reusable technique, fix, or pattern worth capturing as a new skill") + } + if caps.UpdateSkills || caps.DeleteSkills { + reasons = append(reasons, "- an existing skill you used this turn proved wrong, incomplete, or no longer useful") + } + + var stores []string + if caps.WriteMemory { + stores = append(stores, "memory") + } + if caps.CreateSkills || caps.UpdateSkills || caps.DeleteSkills { + stores = append(stores, "skills") + } + saved := strings.Join(stores, " or ") + + desc := "Reflect on the work you just completed this turn and persist any durable learning.\n\n" + + "Call this only when the turn produced something worth carrying into FUTURE sessions:\n" + + strings.Join(reasons, "\n") + "\n\n" + + "A background reviewer then decides what — if anything — to save (into " + saved + + "); you never write " + saved + " yourself. Do NOT call it for routine work, one-off " + + "task state, transient errors, or environment-specific noise. Most turns warrant no " + + "call — when in doubt, don't." + + return core.ToolSchema{ + Name: tool.ToolEvolve, + Description: desc, + Parameters: map[string]any{ + "type": "object", + "properties": map[string]any{ + "reason": map[string]any{ + "type": "string", + "description": "One short clause naming what is worth learning from this turn.", + }, + }, + }, + } +} + +// EvolveTool is the model-facing trigger for a self-learning review. +type EvolveTool struct{} + +func (t *EvolveTool) Name() string { return "Evolve" } +func (t *EvolveTool) Description() string { + return "Request a self-learning review of the current turn" +} +func (t *EvolveTool) Icon() string { return IconEvolve } + +// Execute just acknowledges: the app observes this call and considers a review +// at turn end, so the tool has nothing to do but confirm and echo the model's +// reason. The wording is "flagged", not "queued" — the review may still be +// skipped (turn cancelled, a prior review in flight, permissions leave the +// pass nothing to do). +func (t *EvolveTool) Execute(ctx context.Context, params map[string]any, cwd string) toolresult.ToolResult { + reason := tool.GetString(params, "reason") + out := "Flagged this turn for a self-learning review at turn end." + if reason != "" { + out = "Flagged this turn for self-learning review: " + reason + } + return toolresult.ToolResult{ + Success: true, + Output: out, + Metadata: toolresult.ResultMetadata{ + Title: "Evolve", + Icon: IconEvolve, + Subtitle: "self-learning flagged", + }, + } +} + +func init() { + tool.Register(&EvolveTool{}) +} diff --git a/internal/tool/evolve/schema_test.go b/internal/tool/evolve/schema_test.go new file mode 100644 index 00000000..b9a0c4a4 --- /dev/null +++ b/internal/tool/evolve/schema_test.go @@ -0,0 +1,49 @@ +package evolve + +import ( + "strings" + "testing" +) + +// TestCapabilitiesActive confirms the all-off zero value reads inactive (the +// app then omits the tool entirely) and any single capability activates it. +func TestCapabilitiesActive(t *testing.T) { + if (Capabilities{}).Active() { + t.Fatal("zero capabilities must be inactive") + } + for _, c := range []Capabilities{ + {CreateSkills: true}, {UpdateSkills: true}, {DeleteSkills: true}, {WriteMemory: true}, + } { + if !c.Active() { + t.Fatalf("%+v should be active", c) + } + } +} + +// TestSchemaDescriptionIsDynamic confirms the description only advertises the +// enabled capabilities — memory-off never mentions memory, and skill-off never +// invites capturing/refining a skill. +func TestSchemaDescriptionIsDynamic(t *testing.T) { + memOnly := Schema(Capabilities{WriteMemory: true}).Description + if !strings.Contains(memOnly, "remembering") || strings.Contains(memOnly, "skill") { + t.Fatalf("memory-only description should mention memory, not skills:\n%s", memOnly) + } + + skillOnly := Schema(Capabilities{CreateSkills: true, UpdateSkills: true, DeleteSkills: true}).Description + if strings.Contains(skillOnly, "remember") { + t.Fatalf("skill-only description must not mention memory:\n%s", skillOnly) + } + if !strings.Contains(skillOnly, "new skill") { + t.Fatalf("create-enabled description should mention capturing a new skill:\n%s", skillOnly) + } + + // Delete-only (no create/update): no "new skill" invite, but the used-skill + // line is present so the model can flag a skill worth retiring. + delOnly := Schema(Capabilities{DeleteSkills: true}).Description + if strings.Contains(delOnly, "new skill") { + t.Fatalf("delete-only description must not invite creating skills:\n%s", delOnly) + } + if !strings.Contains(delOnly, "existing skill you used") { + t.Fatalf("delete-only description should reference the used skill:\n%s", delOnly) + } +} diff --git a/internal/tool/extra_tools_test.go b/internal/tool/extra_tools_test.go new file mode 100644 index 00000000..58b5135a --- /dev/null +++ b/internal/tool/extra_tools_test.go @@ -0,0 +1,44 @@ +package tool + +import ( + "testing" + + "github.com/genai-io/san/internal/core" +) + +func schemasContain(schemas []core.ToolSchema, name string) bool { + for _, s := range schemas { + if s.Name == name { + return true + } + } + return false +} + +// TestExtraToolsPlumbing confirms caller-supplied schemas are appended to the +// set (this is how the main agent injects the Evolve trigger), absent by +// default, and subject to the same disabled filtering as built-in tools. +func TestExtraToolsPlumbing(t *testing.T) { + extra := core.ToolSchema{Name: "Evolve", Description: "extra"} + + if schemasContain(GetToolSchemasWith(SchemaOptions{}), "Evolve") { + t.Fatal("no extra tool supplied — Evolve must be absent from the default schema set") + } + if !schemasContain(GetToolSchemasWith(SchemaOptions{ExtraTools: []core.ToolSchema{extra}}), "Evolve") { + t.Fatal("a supplied extra tool must appear in the schema set") + } + + if schemasContain((&Set{}).Tools(), "Evolve") { + t.Fatal("Evolve must be absent from a default Set") + } + if !schemasContain((&Set{ExtraTools: []core.ToolSchema{extra}}).Tools(), "Evolve") { + t.Fatal("a Set's extra tools must appear in its toolset") + } + disabled := &Set{ + ExtraTools: []core.ToolSchema{extra}, + Disabled: map[string]bool{"Evolve": true}, + } + if schemasContain(disabled.Tools(), "Evolve") { + t.Fatal("extra tools must pass through the disabled filter like any other tool") + } +} diff --git a/internal/tool/perm/decision.go b/internal/tool/perm/decision.go index 97f92443..b94cf675 100644 --- a/internal/tool/perm/decision.go +++ b/internal/tool/perm/decision.go @@ -61,6 +61,9 @@ var safeTools = func() map[string]bool { "TaskUpdate": true, "AskUserQuestion": true, "CronList": true, + // Evolve only queues a background self-learning review — it writes + // nothing itself, so it auto-allows like the task/question tools. + "Evolve": true, } for name := range readOnlyTools { m[name] = true diff --git a/internal/tool/perm/decision_test.go b/internal/tool/perm/decision_test.go index c4ed5ac7..b526a316 100644 --- a/internal/tool/perm/decision_test.go +++ b/internal/tool/perm/decision_test.go @@ -20,7 +20,7 @@ func TestIsReadOnlyTool(t *testing.T) { func TestIsSafeTool(t *testing.T) { safe := []string{"TaskCreate", "TaskGet", "TaskList", "TaskUpdate", - "AskUserQuestion", "LSP"} + "AskUserQuestion", "LSP", "Evolve"} for _, name := range safe { if !IsSafeTool(name) { t.Errorf("IsSafeTool(%q) = false, want true", name) diff --git a/internal/tool/registry/all.go b/internal/tool/registry/all.go index 32691cb5..693165dd 100644 --- a/internal/tool/registry/all.go +++ b/internal/tool/registry/all.go @@ -4,6 +4,7 @@ package registry import ( _ "github.com/genai-io/san/internal/tool/agent" _ "github.com/genai-io/san/internal/tool/cron" + _ "github.com/genai-io/san/internal/tool/evolve" _ "github.com/genai-io/san/internal/tool/fs" _ "github.com/genai-io/san/internal/tool/mode" _ "github.com/genai-io/san/internal/tool/skill" diff --git a/internal/tool/schema.go b/internal/tool/schema.go index f7df284c..69a4c062 100644 --- a/internal/tool/schema.go +++ b/internal/tool/schema.go @@ -24,6 +24,8 @@ const ( ToolExitWorktree = "ExitWorktree" ToolAskUserQuestion = "AskUserQuestion" + + ToolEvolve = "Evolve" ) // IsAgentToolName reports whether the tool name represents an agent-like worker tool. @@ -41,6 +43,13 @@ func IsAgentToolName(name string) bool { type SchemaOptions struct { MCPTools func() []core.ToolSchema AgentDirectory func() string + + // ExtraTools are caller-supplied schemas appended to the registered set — + // the hook for conditionally-present tools whose schema the caller builds + // (e.g. the self-learning Evolve trigger, built by tool/evolve.Schema and + // injected only by the main agent). They pass through the same + // disabled/allow filtering as every other tool. + ExtraTools []core.ToolSchema } // GetToolSchemas returns core.ToolSchema definitions for all registered tools @@ -65,6 +74,8 @@ func GetToolSchemasWith(opts SchemaOptions) []core.ToolSchema { tools = append(tools, cronToolSchemas...) tools = append(tools, worktreeToolSchemas...) + tools = append(tools, opts.ExtraTools...) + if opts.MCPTools != nil { tools = append(tools, opts.MCPTools()...) } diff --git a/internal/tool/set.go b/internal/tool/set.go index 63bf4138..b2bbbc18 100644 --- a/internal/tool/set.go +++ b/internal/tool/set.go @@ -24,6 +24,7 @@ type Set struct { Allow []string // agent allow list (nil = all tools, non-nil = only these) Disallow []string // agent deny list (excluded after allow filtering) IsAgent bool // true for subagent tool sets (excludes parent-only tools) + ExtraTools []core.ToolSchema // caller-built conditional tools (e.g. Evolve; main agent only) disallowSet map[string]bool // eagerly-initialized normalized lookup cache for Disallow } @@ -53,6 +54,7 @@ func (s *Set) defaultTools() []core.ToolSchema { tools := GetToolSchemasWith(SchemaOptions{ MCPTools: s.MCP, AgentDirectory: s.AgentDirectory, + ExtraTools: s.ExtraTools, }) filtered := make([]core.ToolSchema, 0, len(tools))