fix(agent): invalidate system prompt cache for global/builtin skills

pkg/agent/context.go

@@ -170,10 +170,9 @@ func (cb *ContextBuilder) InvalidateCache() {
 	logger.DebugCF("agent", "System prompt cache invalidated", nil)
 }
 
-// sourcePaths returns the workspace source file paths tracked for cache
+// sourcePaths returns non-skill workspace source files tracked for cache
-// invalidation (bootstrap files + memory). The skills directory is handled
+// invalidation (bootstrap files + memory). Skill roots are handled separately
-// separately in sourceFilesChangedLocked because it requires both directory-
+// because they require both directory-level and recursive file-level checks.
-// level and recursive file-level mtime checks.
 func (cb *ContextBuilder) sourcePaths() []string {
 	return []string{
 		filepath.Join(cb.workspace, "AGENTS.md"),
@@ -184,6 +183,20 @@ func (cb *ContextBuilder) sourcePaths() []string {
 	}
 }
 
+// skillRoots returns all skill root directories that can affect
+// BuildSkillsSummary output (workspace/global/builtin).
+func (cb *ContextBuilder) skillRoots() []string {
+	if cb.skillsLoader == nil {
+		return []string{filepath.Join(cb.workspace, "skills")}
+	}
+
+	roots := cb.skillsLoader.SkillRoots()
+	if len(roots) == 0 {
+		return []string{filepath.Join(cb.workspace, "skills")}
+	}
+	return roots
+}
+
 // cacheBaseline holds the file existence snapshot and the latest observed
 // mtime across all tracked paths. Used as the cache reference point.
 type cacheBaseline struct {
@@ -195,10 +208,10 @@ type cacheBaseline struct {
 // the latest mtime across all tracked files + skills directory contents.
 // Called under write lock when the cache is built.
 func (cb *ContextBuilder) buildCacheBaseline() cacheBaseline {
-	skillsDir := filepath.Join(cb.workspace, "skills")
+	skillRoots := cb.skillRoots()
 
-	// All paths whose existence we track: source files + skills dir.
+	// All paths whose existence we track: source files + all skill roots.
-	allPaths := append(cb.sourcePaths(), skillsDir)
+	allPaths := append(cb.sourcePaths(), skillRoots...)
 
 	existed := make(map[string]bool, len(allPaths))
 	var maxMtime time.Time
@@ -211,17 +224,19 @@ func (cb *ContextBuilder) buildCacheBaseline() cacheBaseline {
 		}
 	}
 
-	// Walk skills files to capture their mtimes too.
+	// Walk all skill roots recursively to capture skill file mtimes too.
 	// Use os.Stat (not d.Info) to match the stat method used in
 	// fileChangedSince / skillFilesModifiedSince for consistency.
-	_ = filepath.WalkDir(skillsDir, func(path string, d fs.DirEntry, walkErr error) error {
+	for _, root := range skillRoots {
-		if walkErr == nil && !d.IsDir() {
+		_ = filepath.WalkDir(root, func(path string, d fs.DirEntry, walkErr error) error {
-			if info, err := os.Stat(path); err == nil && info.ModTime().After(maxMtime) {
+			if walkErr == nil && !d.IsDir() {
-				maxMtime = info.ModTime()
+				if info, err := os.Stat(path); err == nil && info.ModTime().After(maxMtime) {
+					maxMtime = info.ModTime()
+				}
 			}
-		}
+			return nil
-		return nil
+		})
-	})
+	}
 
 	// If no tracked files exist yet (empty workspace), maxMtime is zero.
 	// Use a very old non-zero time so that:
@@ -255,22 +270,18 @@ func (cb *ContextBuilder) sourceFilesChangedLocked() bool {
 		}
 	}
 
-	// --- Skills directory (handled separately from sourcePaths) ---
+	// --- Skill roots (workspace/global/builtin) ---
 	//
-	// 1. Creation/deletion: tracked via existedAtCache, same as bootstrap files.
+	// For each root:
-	skillsDir := filepath.Join(cb.workspace, "skills")
+	// 1. Creation/deletion and directory mtime changes are tracked by fileChangedSince.
-	if cb.fileChangedSince(skillsDir) {
+	// 2. Content-only edits inside the tree are tracked by recursive file mtime checks.
-		return true
+	for _, root := range cb.skillRoots() {
-	}
+		if cb.fileChangedSince(root) {
-
+			return true
-	// 2. Structural changes (add/remove entries inside the dir) are reflected
+		}
-	//    in the directory's own mtime, which fileChangedSince already checks.
+		if skillFilesModifiedSince(root, cb.cachedAt) {
-	//
+			return true
-	// 3. Content-only edits to files inside skills/ do NOT update the parent
+		}
-	//    directory mtime on most filesystems, so we recursively walk to check
-	//    individual file mtimes at any nesting depth.
-	if skillFilesModifiedSince(skillsDir, cb.cachedAt) {
-		return true
 	}
 
 	return false

pkg/agent/context_cache_test.go

@@ -383,6 +383,128 @@ Updated content.`
 	}
 }
 
+// TestGlobalSkillFileContentChange verifies that modifying a global skill
+// (~/.picoclaw/skills) invalidates the cached system prompt.
+func TestGlobalSkillFileContentChange(t *testing.T) {
+	tmpHome := t.TempDir()
+	t.Setenv("HOME", tmpHome)
+
+	tmpDir := setupWorkspace(t, nil)
+	defer os.RemoveAll(tmpDir)
+
+	globalSkillPath := filepath.Join(tmpHome, ".picoclaw", "skills", "global-skill", "SKILL.md")
+	if err := os.MkdirAll(filepath.Dir(globalSkillPath), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	v1 := `---
+name: global-skill
+description: global-v1
+---
+# Global Skill v1`
+	if err := os.WriteFile(globalSkillPath, []byte(v1), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	cb := NewContextBuilder(tmpDir)
+	sp1 := cb.BuildSystemPromptWithCache()
+	if !strings.Contains(sp1, "global-v1") {
+		t.Fatal("expected initial prompt to contain global skill description")
+	}
+
+	v2 := `---
+name: global-skill
+description: global-v2
+---
+# Global Skill v2`
+	if err := os.WriteFile(globalSkillPath, []byte(v2), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	future := time.Now().Add(2 * time.Second)
+	os.Chtimes(globalSkillPath, future, future)
+
+	cb.systemPromptMutex.RLock()
+	changed := cb.sourceFilesChangedLocked()
+	cb.systemPromptMutex.RUnlock()
+	if !changed {
+		t.Fatal("sourceFilesChangedLocked() should detect global skill file content change")
+	}
+
+	sp2 := cb.BuildSystemPromptWithCache()
+	if !strings.Contains(sp2, "global-v2") {
+		t.Error("rebuilt prompt should contain updated global skill description")
+	}
+	if sp1 == sp2 {
+		t.Error("cache should be invalidated when global skill file content changes")
+	}
+}
+
+// TestBuiltinSkillFileContentChange verifies that modifying a builtin skill
+// ({cwd}/skills) invalidates the cached system prompt.
+func TestBuiltinSkillFileContentChange(t *testing.T) {
+	tmpHome := t.TempDir()
+	t.Setenv("HOME", tmpHome)
+
+	tmpDir := setupWorkspace(t, nil)
+	defer os.RemoveAll(tmpDir)
+
+	builtinRoot := t.TempDir()
+	oldWD, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := os.Chdir(builtinRoot); err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() {
+		_ = os.Chdir(oldWD)
+	})
+
+	builtinSkillPath := filepath.Join(builtinRoot, "skills", "builtin-skill", "SKILL.md")
+	if err := os.MkdirAll(filepath.Dir(builtinSkillPath), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	v1 := `---
+name: builtin-skill
+description: builtin-v1
+---
+# Builtin Skill v1`
+	if err := os.WriteFile(builtinSkillPath, []byte(v1), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	cb := NewContextBuilder(tmpDir)
+	sp1 := cb.BuildSystemPromptWithCache()
+	if !strings.Contains(sp1, "builtin-v1") {
+		t.Fatal("expected initial prompt to contain builtin skill description")
+	}
+
+	v2 := `---
+name: builtin-skill
+description: builtin-v2
+---
+# Builtin Skill v2`
+	if err := os.WriteFile(builtinSkillPath, []byte(v2), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	future := time.Now().Add(2 * time.Second)
+	os.Chtimes(builtinSkillPath, future, future)
+
+	cb.systemPromptMutex.RLock()
+	changed := cb.sourceFilesChangedLocked()
+	cb.systemPromptMutex.RUnlock()
+	if !changed {
+		t.Fatal("sourceFilesChangedLocked() should detect builtin skill file content change")
+	}
+
+	sp2 := cb.BuildSystemPromptWithCache()
+	if !strings.Contains(sp2, "builtin-v2") {
+		t.Error("rebuilt prompt should contain updated builtin skill description")
+	}
+	if sp1 == sp2 {
+		t.Error("cache should be invalidated when builtin skill file content changes")
+	}
+}
+
 // TestConcurrentBuildSystemPromptWithCache verifies that multiple goroutines
 // can safely call BuildSystemPromptWithCache concurrently without producing
 // empty results, panics, or data races.

pkg/skills/loader.go

@@ -64,6 +64,28 @@ type SkillsLoader struct {
 	builtinSkills   string // builtin skills
 }
 
+// SkillRoots returns all unique skill root directories used by this loader.
+// The order follows resolution priority: workspace > global > builtin.
+func (sl *SkillsLoader) SkillRoots() []string {
+	roots := []string{sl.workspaceSkills, sl.globalSkills, sl.builtinSkills}
+	seen := make(map[string]struct{}, len(roots))
+	out := make([]string, 0, len(roots))
+
+	for _, root := range roots {
+		if strings.TrimSpace(root) == "" {
+			continue
+		}
+		clean := filepath.Clean(root)
+		if _, ok := seen[clean]; ok {
+			continue
+		}
+		seen[clean] = struct{}{}
+		out = append(out, clean)
+	}
+
+	return out
+}
+
 func NewSkillsLoader(workspace string, globalSkills string, builtinSkills string) *SkillsLoader {
 	return &SkillsLoader{
 		workspace:       workspace,