refactor: move skill catalog handling to dynamic context and update tests

pkg/agent/context.go

@@ -202,24 +202,6 @@ func (cb *ContextBuilder) BuildSystemPromptParts() []PromptPart {
 		})
 	}
 
-	// Skills - show summary, AI can read full content with read_file tool
-	skillsSummary := cb.skillsLoader.BuildSkillsSummary()
-	if skillsSummary != "" {
-		add(PromptPart{
-			ID:     "capability.skill_catalog",
-			Layer:  PromptLayerCapability,
-			Slot:   PromptSlotSkillCatalog,
-			Source: PromptSource{ID: PromptSourceSkillCatalog, Name: "skill:index"},
-			Title:  "skill catalog",
-			Content: fmt.Sprintf(`# Skills
-
-The following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool.
-
-%s`, skillsSummary),
-			Stable: true,
-			Cache:  PromptCacheEphemeral,
-		})
-	}
 
 	// Memory context
 	memoryContext := cb.memory.GetMemoryContext()
@@ -314,6 +296,13 @@ func (cb *ContextBuilder) EstimateSystemTokens(summary string, activeSkills []st
 
 	totalChars := utf8.RuneCountInString(staticPrompt) + dynamicContextChars
 
+	// Skill catalog is no longer in the static prompt; add it to the estimate
+	// (EstimateSystemTokens assumes a non-continuation turn).
+	if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" {
+		totalChars += utf8.RuneCountInString(skillsSummary) + 80 // header overhead
+		totalChars += 7                                           // separator
+	}
+
 	if skillsText := cb.buildActiveSkillsContext(activeSkills); skillsText != "" {
 		totalChars += utf8.RuneCountInString(skillsText)
 		totalChars += 7 // separator \n\n---\n\n
@@ -701,6 +690,28 @@ func (cb *ContextBuilder) BuildMessagesFromPrompt(req PromptBuildRequest) []prov
 		}, &providers.CacheControl{Type: "ephemeral"}),
 	}
 
+	// Skip the skill catalog on tool-call continuations: the LLM already saw
+	// it in the initial turn request and doesn't need it re-sent for every
+	// intermediate tool round-trip. This saves significant tokens on providers
+	// without prompt caching (OpenAI-compat).
+	isToolContinuation := len(req.History) > 0 && req.History[len(req.History)-1].Role == "tool"
+	if !isToolContinuation {
+		if skillsSummary := cb.skillsLoader.BuildSkillsSummary(); skillsSummary != "" {
+			catalogPart := PromptPart{
+				ID:    "capability.skill_catalog",
+				Layer: PromptLayerCapability,
+				Slot:  PromptSlotSkillCatalog,
+				Source: PromptSource{ID: PromptSourceSkillCatalog, Name: "skill:index"},
+				Title: "skill catalog",
+				Content: fmt.Sprintf("# Skills\n\nThe following skills extend your capabilities. To use a skill, read its SKILL.md file using the read_file tool.\n\n%s", skillsSummary),
+				Stable: true,
+				Cache:  PromptCacheEphemeral,
+			}
+			stringParts = append(stringParts, catalogPart.Content)
+			contentBlocks = append(contentBlocks, promptContentBlock(catalogPart, &providers.CacheControl{Type: "ephemeral"}))
+		}
+	}
+
 	promptParts := append([]PromptPart(nil), req.Overlays...)
 	promptParts = append(promptParts, cb.buildActiveSkillsPromptParts(req.ActiveSkills)...)
 	if contributedParts, err := cb.promptRegistryOrDefault().Collect(context.Background(), req); err != nil {

pkg/agent/context_cache_test.go

@@ -31,6 +31,16 @@ func setupWorkspace(t *testing.T, files map[string]string) string {
 	return tmpDir
 }
 
+// systemPromptFromMessages extracts the Content of the first system message.
+func systemPromptFromMessages(msgs []providers.Message) string {
+	for _, m := range msgs {
+		if m.Role == "system" {
+			return m.Content
+		}
+	}
+	return ""
+}
+
 // TestSingleSystemMessage verifies that BuildMessages always produces exactly one
 // system message regardless of summary/history variations.
 // Fix: multiple system messages break Anthropic (top-level system param) and
@@ -468,8 +478,9 @@ description: global-v1
 	}
 
 	cb := NewContextBuilder(tmpDir)
-	sp1 := cb.BuildSystemPromptWithCache()
+	// Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt.
-	if !strings.Contains(sp1, "global-v1") {
+	sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
+	if !strings.Contains(sysMsg1, "global-v1") {
 		t.Fatal("expected initial prompt to contain global skill description")
 	}
 
@@ -493,11 +504,11 @@ description: global-v2
 		t.Fatal("sourceFilesChangedLocked() should detect global skill file content change")
 	}
 
-	sp2 := cb.BuildSystemPromptWithCache()
+	sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
-	if !strings.Contains(sp2, "global-v2") {
+	if !strings.Contains(sysMsg2, "global-v2") {
 		t.Error("rebuilt prompt should contain updated global skill description")
 	}
-	if sp1 == sp2 {
+	if sysMsg1 == sysMsg2 {
 		t.Error("cache should be invalidated when global skill file content changes")
 	}
 }
@@ -528,8 +539,9 @@ description: builtin-v1
 	}
 
 	cb := NewContextBuilder(tmpDir)
-	sp1 := cb.BuildSystemPromptWithCache()
+	// Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt.
-	if !strings.Contains(sp1, "builtin-v1") {
+	sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
+	if !strings.Contains(sysMsg1, "builtin-v1") {
 		t.Fatal("expected initial prompt to contain builtin skill description")
 	}
 
@@ -553,11 +565,11 @@ description: builtin-v2
 		t.Fatal("sourceFilesChangedLocked() should detect builtin skill file content change")
 	}
 
-	sp2 := cb.BuildSystemPromptWithCache()
+	sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
-	if !strings.Contains(sp2, "builtin-v2") {
+	if !strings.Contains(sysMsg2, "builtin-v2") {
 		t.Error("rebuilt prompt should contain updated builtin skill description")
 	}
-	if sp1 == sp2 {
+	if sysMsg1 == sysMsg2 {
 		t.Error("cache should be invalidated when builtin skill file content changes")
 	}
 }
@@ -575,8 +587,9 @@ description: delete-me-v1
 	defer os.RemoveAll(tmpDir)
 
 	cb := NewContextBuilder(tmpDir)
-	sp1 := cb.BuildSystemPromptWithCache()
+	// Skill catalog is injected per-request, not in the static cache; check via BuildMessagesFromPrompt.
-	if !strings.Contains(sp1, "delete-me-v1") {
+	sysMsg1 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
+	if !strings.Contains(sysMsg1, "delete-me-v1") {
 		t.Fatal("expected initial prompt to contain skill description")
 	}
 
@@ -592,11 +605,11 @@ description: delete-me-v1
 		t.Fatal("sourceFilesChangedLocked() should detect deleted skill file")
 	}
 
-	sp2 := cb.BuildSystemPromptWithCache()
+	sysMsg2 := systemPromptFromMessages(cb.BuildMessagesFromPrompt(PromptBuildRequest{}))
-	if strings.Contains(sp2, "delete-me-v1") {
+	if strings.Contains(sysMsg2, "delete-me-v1") {
 		t.Error("rebuilt prompt should not contain deleted skill description")
 	}
-	if sp1 == sp2 {
+	if sysMsg1 == sysMsg2 {
 		t.Error("cache should be invalidated when skill file is deleted")
 	}
 }