From bf8fce7cf64c431a3cf0c150ef5b2ee99c277ea2 Mon Sep 17 00:00:00 2001 From: Anu S Pillai Date: Mon, 9 Mar 2026 01:12:48 +0530 Subject: [PATCH] fix(agent): auto-inject SKILL.md into LLM context when user references a skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire up the existing but unused LoadSkillsForContext() so that skill instructions are automatically injected into the system prompt when the user's message references an installed skill by name. Previously the LLM only saw a skills summary listing names and file paths, with instructions to "read the SKILL.md using read_file". Smaller and local models (e.g. Ollama) do not reliably decide to call read_file on their own, leaving skills effectively broken. Changes: - Add MatchSkillsInMessage() for case-insensitive whole-word matching of skill names against the user message - Add LoadSkillContext() wrapper to surface LoadSkillsForContext() - Extend BuildMessages() with a variadic skillContext parameter (backward-compatible — all existing callers are unchanged) - Inject matched skill content as a dynamic content block in the system message under "Active Skill Instructions" - Wire the matching and injection into runAgentLoop() before BuildMessages() is called - Add comprehensive tests for matching, boundary detection, context injection, and nil-safety Fixes #1249 Made-with: Cursor --- pkg/agent/context.go | 68 +++++++++ pkg/agent/context_skills_test.go | 235 +++++++++++++++++++++++++++++++ pkg/agent/loop.go | 10 ++ 3 files changed, 313 insertions(+) create mode 100644 pkg/agent/context_skills_test.go diff --git a/pkg/agent/context.go b/pkg/agent/context.go index 5a84c45e2..fb03979e5 100644 --- a/pkg/agent/context.go +++ b/pkg/agent/context.go @@ -478,6 +478,7 @@ func (cb *ContextBuilder) BuildMessages( currentMessage string, media []string, channel, chatID string, + skillContext ...string, ) []providers.Message { messages := []providers.Message{} @@ -520,6 +521,15 @@ func (cb *ContextBuilder) BuildMessages( contentBlocks = append(contentBlocks, providers.ContentBlock{Type: "text", Text: summaryText}) } + if len(skillContext) > 0 && skillContext[0] != "" { + skillBlock := "# Active Skill Instructions\n\n" + + "The user's message references the following skill(s). " + + "Follow these instructions to fulfill the request.\n\n" + + skillContext[0] + stringParts = append(stringParts, skillBlock) + contentBlocks = append(contentBlocks, providers.ContentBlock{Type: "text", Text: skillBlock}) + } + fullSystemPrompt := strings.Join(stringParts, "\n\n---\n\n") // Log system prompt summary for debugging (debug mode only). @@ -731,3 +741,61 @@ func (cb *ContextBuilder) GetSkillsInfo() map[string]any { "names": skillNames, } } + +// MatchSkillsInMessage returns the names of installed skills that are +// referenced in the user message. Matching is case-insensitive and looks +// for the skill name as a whole word (bounded by non-alphanumeric/hyphen +// characters or string edges) to avoid false positives from partial +// matches inside unrelated words. +func (cb *ContextBuilder) MatchSkillsInMessage(message string) []string { + if cb.skillsLoader == nil { + return nil + } + + allSkills := cb.skillsLoader.ListSkills() + if len(allSkills) == 0 { + return nil + } + + lowerMsg := strings.ToLower(message) + var matched []string + + for _, s := range allSkills { + lowerName := strings.ToLower(s.Name) + idx := 0 + for { + pos := strings.Index(lowerMsg[idx:], lowerName) + if pos < 0 { + break + } + absPos := idx + pos + endPos := absPos + len(lowerName) + + beforeOK := absPos == 0 || !isSkillNameChar(lowerMsg[absPos-1]) + afterOK := endPos == len(lowerMsg) || !isSkillNameChar(lowerMsg[endPos]) + + if beforeOK && afterOK { + matched = append(matched, s.Name) + break + } + idx = endPos + } + } + + return matched +} + +// LoadSkillContext loads the full SKILL.md content for the given skill names +// via the underlying SkillsLoader. +func (cb *ContextBuilder) LoadSkillContext(skillNames []string) string { + if cb.skillsLoader == nil { + return "" + } + return cb.skillsLoader.LoadSkillsForContext(skillNames) +} + +// isSkillNameChar returns true for characters that can appear inside a skill +// name (alphanumeric and hyphen). Used for whole-word boundary detection. +func isSkillNameChar(c byte) bool { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') || c == '-' +} diff --git a/pkg/agent/context_skills_test.go b/pkg/agent/context_skills_test.go new file mode 100644 index 000000000..47a8d86ac --- /dev/null +++ b/pkg/agent/context_skills_test.go @@ -0,0 +1,235 @@ +package agent + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// createTestSkill creates a skill directory with a SKILL.md containing frontmatter. +func createTestSkill(t *testing.T, skillsDir, dirName, name, description, body string) { + t.Helper() + dir := filepath.Join(skillsDir, dirName) + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatal(err) + } + content := "---\nname: " + name + "\ndescription: " + description + "\n---\n\n" + body + if err := os.WriteFile(filepath.Join(dir, "SKILL.md"), []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func TestMatchSkillsInMessage_Basic(t *testing.T) { + tmpDir := setupWorkspace(t, nil) + defer os.RemoveAll(tmpDir) + + createTestSkill(t, filepath.Join(tmpDir, "skills"), "news-summary", "news-summary", "Summarize news", "Read RSS feeds") + createTestSkill(t, filepath.Join(tmpDir, "skills"), "weather", "weather", "Get weather info", "Check weather API") + + cb := NewContextBuilder(tmpDir) + + tests := []struct { + name string + message string + expected []string + }{ + { + name: "exact skill name", + message: "use the news-summary skill", + expected: []string{"news-summary"}, + }, + { + name: "case insensitive", + message: "Use the News-Summary skill please", + expected: []string{"news-summary"}, + }, + { + name: "multiple skills", + message: "get the weather and run news-summary", + expected: []string{"news-summary", "weather"}, + }, + { + name: "no match", + message: "hello how are you", + expected: nil, + }, + { + name: "empty message", + message: "", + expected: nil, + }, + { + name: "skill name at start", + message: "news-summary please run it", + expected: []string{"news-summary"}, + }, + { + name: "skill name at end", + message: "please run news-summary", + expected: []string{"news-summary"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matched := cb.MatchSkillsInMessage(tc.message) + if tc.expected == nil { + if len(matched) != 0 { + t.Errorf("expected no matches, got %v", matched) + } + return + } + if len(matched) != len(tc.expected) { + t.Fatalf("expected %d matches %v, got %d: %v", len(tc.expected), tc.expected, len(matched), matched) + } + for _, exp := range tc.expected { + found := false + for _, m := range matched { + if m == exp { + found = true + break + } + } + if !found { + t.Errorf("expected match %q not found in %v", exp, matched) + } + } + }) + } +} + +func TestMatchSkillsInMessage_WholeWordBoundary(t *testing.T) { + tmpDir := setupWorkspace(t, nil) + defer os.RemoveAll(tmpDir) + + createTestSkill(t, filepath.Join(tmpDir, "skills"), "git", "git", "Git operations", "Run git commands") + + cb := NewContextBuilder(tmpDir) + + tests := []struct { + name string + message string + match bool + }{ + {"standalone", "use git to commit", true}, + {"with punctuation", "run git, then push", true}, + {"in parentheses", "tools (git) are available", true}, + {"partial word github", "check github for updates", false}, + {"partial word digit", "use digit recognition", false}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + matched := cb.MatchSkillsInMessage(tc.message) + if tc.match && len(matched) == 0 { + t.Errorf("expected match for %q but got none", tc.message) + } + if !tc.match && len(matched) > 0 { + t.Errorf("expected no match for %q but got %v", tc.message, matched) + } + }) + } +} + +func TestMatchSkillsInMessage_NilLoader(t *testing.T) { + cb := &ContextBuilder{skillsLoader: nil} + matched := cb.MatchSkillsInMessage("use weather skill") + if len(matched) != 0 { + t.Errorf("expected no matches with nil loader, got %v", matched) + } +} + +func TestLoadSkillContext(t *testing.T) { + tmpDir := setupWorkspace(t, nil) + defer os.RemoveAll(tmpDir) + + createTestSkill(t, filepath.Join(tmpDir, "skills"), "weather", "weather", "Get weather", "Call the weather API with the user's location.") + + cb := NewContextBuilder(tmpDir) + + ctx := cb.LoadSkillContext([]string{"weather"}) + if ctx == "" { + t.Fatal("expected non-empty skill context") + } + if !strings.Contains(ctx, "weather API") { + t.Errorf("skill context should contain skill body, got: %s", ctx) + } +} + +func TestLoadSkillContext_NilLoader(t *testing.T) { + cb := &ContextBuilder{skillsLoader: nil} + ctx := cb.LoadSkillContext([]string{"weather"}) + if ctx != "" { + t.Errorf("expected empty context with nil loader, got: %s", ctx) + } +} + +func TestBuildMessages_WithSkillContext(t *testing.T) { + tmpDir := setupWorkspace(t, nil) + defer os.RemoveAll(tmpDir) + + createTestSkill(t, filepath.Join(tmpDir, "skills"), "weather", "weather", "Get weather", "Call the weather API.") + + cb := NewContextBuilder(tmpDir) + skillCtx := cb.LoadSkillContext([]string{"weather"}) + + messages := cb.BuildMessages(nil, "", "check weather", nil, "telegram", "123", skillCtx) + + if len(messages) < 2 { + t.Fatalf("expected at least 2 messages (system + user), got %d", len(messages)) + } + + systemContent := messages[0].Content + if !strings.Contains(systemContent, "Active Skill Instructions") { + t.Error("system prompt should contain 'Active Skill Instructions' when skill context is provided") + } + if !strings.Contains(systemContent, "weather API") { + t.Error("system prompt should contain the skill body content") + } +} + +func TestBuildMessages_WithoutSkillContext(t *testing.T) { + tmpDir := setupWorkspace(t, nil) + defer os.RemoveAll(tmpDir) + + cb := NewContextBuilder(tmpDir) + + messages := cb.BuildMessages(nil, "", "hello", nil, "telegram", "123") + + if len(messages) < 2 { + t.Fatalf("expected at least 2 messages, got %d", len(messages)) + } + + systemContent := messages[0].Content + if strings.Contains(systemContent, "Active Skill Instructions") { + t.Error("system prompt should NOT contain skill instructions when no skill context") + } +} + +func TestBuildMessages_EmptySkillContext(t *testing.T) { + tmpDir := setupWorkspace(t, nil) + defer os.RemoveAll(tmpDir) + + cb := NewContextBuilder(tmpDir) + + messages := cb.BuildMessages(nil, "", "hello", nil, "telegram", "123", "") + + systemContent := messages[0].Content + if strings.Contains(systemContent, "Active Skill Instructions") { + t.Error("system prompt should NOT contain skill instructions when skill context is empty string") + } +} + +func TestIsSkillNameChar(t *testing.T) { + for _, c := range "abcxyzABCXYZ0189-" { + if !isSkillNameChar(byte(c)) { + t.Errorf("expected %q to be a skill name char", string(c)) + } + } + for _, c := range " ._/@!,()[]" { + if isSkillNameChar(byte(c)) { + t.Errorf("expected %q to NOT be a skill name char", string(c)) + } + } +} diff --git a/pkg/agent/loop.go b/pkg/agent/loop.go index 235d42fcc..8da52d6a6 100644 --- a/pkg/agent/loop.go +++ b/pkg/agent/loop.go @@ -800,6 +800,15 @@ func (al *AgentLoop) runAgentLoop( history = agent.Sessions.GetHistory(opts.SessionKey) summary = agent.Sessions.GetSummary(opts.SessionKey) } + + // Auto-inject SKILL.md content when the user references an installed skill. + var skillCtx string + if matched := agent.ContextBuilder.MatchSkillsInMessage(opts.UserMessage); len(matched) > 0 { + skillCtx = agent.ContextBuilder.LoadSkillContext(matched) + logger.DebugCF("agent", "Skills matched in user message", + map[string]any{"matched": matched, "context_len": len(skillCtx)}) + } + messages := agent.ContextBuilder.BuildMessages( history, summary, @@ -807,6 +816,7 @@ func (al *AgentLoop) runAgentLoop( opts.Media, opts.Channel, opts.ChatID, + skillCtx, ) // Resolve media:// refs to base64 data URLs (streaming)