fix(agent): address review feedback on skill context injection
- Preserve skill context during compression retry so SKILL.md instructions are not lost when context window is exceeded - Resolve frontmatter names to directory names in LoadSkillContext so skills whose metadata name differs from the directory name are loaded correctly - Add test for metadata name != directory name case Made-with: Cursor
This commit is contained in:
parent
534653228d
commit
e20bfd650a
3 changed files with 56 additions and 7 deletions
|
|
@ -786,12 +786,31 @@ func (cb *ContextBuilder) MatchSkillsInMessage(message string) []string {
|
||||||
}
|
}
|
||||||
|
|
||||||
// LoadSkillContext loads the full SKILL.md content for the given skill names
|
// LoadSkillContext loads the full SKILL.md content for the given skill names
|
||||||
// via the underlying SkillsLoader.
|
// via the underlying SkillsLoader. It resolves frontmatter names to directory
|
||||||
|
// names so that skills whose metadata name differs from the directory name
|
||||||
|
// are loaded correctly.
|
||||||
func (cb *ContextBuilder) LoadSkillContext(skillNames []string) string {
|
func (cb *ContextBuilder) LoadSkillContext(skillNames []string) string {
|
||||||
if cb.skillsLoader == nil {
|
if cb.skillsLoader == nil || len(skillNames) == 0 {
|
||||||
return ""
|
return ""
|
||||||
}
|
}
|
||||||
return cb.skillsLoader.LoadSkillsForContext(skillNames)
|
|
||||||
|
allSkills := cb.skillsLoader.ListSkills()
|
||||||
|
nameToDir := make(map[string]string, len(allSkills))
|
||||||
|
for _, s := range allSkills {
|
||||||
|
dirName := filepath.Base(filepath.Dir(s.Path))
|
||||||
|
nameToDir[strings.ToLower(s.Name)] = dirName
|
||||||
|
}
|
||||||
|
|
||||||
|
resolved := make([]string, 0, len(skillNames))
|
||||||
|
for _, name := range skillNames {
|
||||||
|
if dir, ok := nameToDir[strings.ToLower(name)]; ok {
|
||||||
|
resolved = append(resolved, dir)
|
||||||
|
} else {
|
||||||
|
resolved = append(resolved, name)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return cb.skillsLoader.LoadSkillsForContext(resolved)
|
||||||
}
|
}
|
||||||
|
|
||||||
// isSkillNameChar returns true for characters that can appear inside a skill
|
// isSkillNameChar returns true for characters that can appear inside a skill
|
||||||
|
|
|
||||||
|
|
@ -171,6 +171,35 @@ func TestLoadSkillContext(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestLoadSkillContext_MetadataNameDiffersFromDir(t *testing.T) {
|
||||||
|
tmpDir := setupWorkspace(t, nil)
|
||||||
|
defer os.RemoveAll(tmpDir)
|
||||||
|
|
||||||
|
createTestSkill(
|
||||||
|
t,
|
||||||
|
filepath.Join(tmpDir, "skills"),
|
||||||
|
"weather-skill",
|
||||||
|
"weather",
|
||||||
|
"Get weather",
|
||||||
|
"Call the weather API (metadata name test).",
|
||||||
|
)
|
||||||
|
|
||||||
|
cb := NewContextBuilder(tmpDir)
|
||||||
|
|
||||||
|
matched := cb.MatchSkillsInMessage("use the weather skill")
|
||||||
|
if len(matched) == 0 {
|
||||||
|
t.Fatal("expected to match skill by metadata name 'weather'")
|
||||||
|
}
|
||||||
|
|
||||||
|
ctx := cb.LoadSkillContext(matched)
|
||||||
|
if ctx == "" {
|
||||||
|
t.Fatal("expected non-empty skill context when metadata name differs from directory name")
|
||||||
|
}
|
||||||
|
if !strings.Contains(ctx, "metadata name test") {
|
||||||
|
t.Errorf("skill context should contain skill body, got: %s", ctx)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestLoadSkillContext_NilLoader(t *testing.T) {
|
func TestLoadSkillContext_NilLoader(t *testing.T) {
|
||||||
cb := &ContextBuilder{skillsLoader: nil}
|
cb := &ContextBuilder{skillsLoader: nil}
|
||||||
ctx := cb.LoadSkillContext([]string{"weather"})
|
ctx := cb.LoadSkillContext([]string{"weather"})
|
||||||
|
|
|
||||||
|
|
@ -61,6 +61,7 @@ type processOptions struct {
|
||||||
EnableSummary bool // Whether to trigger summarization
|
EnableSummary bool // Whether to trigger summarization
|
||||||
SendResponse bool // Whether to send response via bus
|
SendResponse bool // Whether to send response via bus
|
||||||
NoHistory bool // If true, don't load session history (for heartbeat)
|
NoHistory bool // If true, don't load session history (for heartbeat)
|
||||||
|
SkillContext string // Injected SKILL.md content for matched skills
|
||||||
}
|
}
|
||||||
|
|
||||||
const (
|
const (
|
||||||
|
|
@ -802,11 +803,10 @@ func (al *AgentLoop) runAgentLoop(
|
||||||
}
|
}
|
||||||
|
|
||||||
// Auto-inject SKILL.md content when the user references an installed skill.
|
// 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 {
|
if matched := agent.ContextBuilder.MatchSkillsInMessage(opts.UserMessage); len(matched) > 0 {
|
||||||
skillCtx = agent.ContextBuilder.LoadSkillContext(matched)
|
opts.SkillContext = agent.ContextBuilder.LoadSkillContext(matched)
|
||||||
logger.DebugCF("agent", "Skills matched in user message",
|
logger.DebugCF("agent", "Skills matched in user message",
|
||||||
map[string]any{"matched": matched, "context_len": len(skillCtx)})
|
map[string]any{"matched": matched, "context_len": len(opts.SkillContext)})
|
||||||
}
|
}
|
||||||
|
|
||||||
messages := agent.ContextBuilder.BuildMessages(
|
messages := agent.ContextBuilder.BuildMessages(
|
||||||
|
|
@ -816,7 +816,7 @@ func (al *AgentLoop) runAgentLoop(
|
||||||
opts.Media,
|
opts.Media,
|
||||||
opts.Channel,
|
opts.Channel,
|
||||||
opts.ChatID,
|
opts.ChatID,
|
||||||
skillCtx,
|
opts.SkillContext,
|
||||||
)
|
)
|
||||||
|
|
||||||
// Resolve media:// refs to base64 data URLs (streaming)
|
// Resolve media:// refs to base64 data URLs (streaming)
|
||||||
|
|
@ -1085,6 +1085,7 @@ func (al *AgentLoop) runLLMIteration(
|
||||||
messages = agent.ContextBuilder.BuildMessages(
|
messages = agent.ContextBuilder.BuildMessages(
|
||||||
newHistory, newSummary, "",
|
newHistory, newSummary, "",
|
||||||
nil, opts.Channel, opts.ChatID,
|
nil, opts.Channel, opts.ChatID,
|
||||||
|
opts.SkillContext,
|
||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue