From de0e8b3bb4aa2e6455fb21b1a07a40114c19c099 Mon Sep 17 00:00:00 2001
From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com>
Date: Sun, 4 Oct 2026 01:50:52 +0530
Subject: [PATCH] perf(skills): memoize the shadow identity decision, not only
 the warning

Gate efficiency finding (Low): resolve_skill_catalog only remembered a
shadowed path once it had warned, so a byte-identical (non-symlink) copy
of a higher-tier skill -- e.g. an external_dirs mirror of local skills --
was re-read and SHA-256-hashed against every owner on each catalog
rebuild / prompt-index miss. Key the memo by (shadowed path, *sorted
higher-tier paths) and record it whether or not a warning fires.
Probe: provably_same_skill calls over 5 resolves of one identical
shadowed copy 5 -> 1. Warn-once behaviour is unchanged.
---
 agent/skill_utils.py            | 15 +++++++++------
 tests/tools/test_skills_tool.py |  2 +-
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/agent/skill_utils.py b/agent/skill_utils.py
index 69d06b08e5..f045427ed5 100644
--- a/agent/skill_utils.py
+++ b/agent/skill_utils.py
@@ -422,7 +422,9 @@ def display_skill_create_dir() -> str:
 TIER_PROJECT, TIER_LOCAL, TIER_CREATE_DIR, TIER_EXTERNAL = range(4)
 # Leading words of every same-tier refusal (skill_view error, preload/cron label) — one spelling.
 AMBIGUOUS_SKILL_PREFIX = "Ambiguous skill name "
-_WARNED_SHADOWS: Set[str] = set()
+# (shadowed path, *sorted higher-tier paths) already judged: the identity check (it hashes both
+# SKILL.md files) and its one-time warning run once per pairing, not on every catalog resolve.
+_SHADOW_CHECKED: Set[Tuple[str, ...]] = set()
 
 
 def get_skill_search_roots(local: Optional[Path] = None, *, include_project: bool = True) -> List[Tuple[int, Path]]:
@@ -502,11 +504,12 @@ def resolve_skill_catalog(entries: List[Dict[str, Any]]) -> List[Dict[str, Any]]
         if higher or winner[name] not in (None, i):  # lower tier, or an identical same-root copy
             e.update(status="shadowed", load_name=None)
             # A symlink view or byte-identical copy of the winner hides nothing worth a warning.
-            if (higher and str(e["path"]) not in _WARNED_SHADOWS
-                    and not any(provably_same_skill([e["path"], out[j]["path"]]) for j in higher)):
-                _WARNED_SHADOWS.add(str(e["path"]))
-                logger.warning("Skill '%s' at %s is shadowed by a higher-precedence copy "
-                               "(project > local > create_dir > external_dirs)", name, e["path"])
+            key = (str(e["path"]), *sorted(str(out[j]["path"]) for j in higher))
+            if higher and key not in _SHADOW_CHECKED:
+                _SHADOW_CHECKED.add(key)
+                if not any(provably_same_skill([e["path"], out[j]["path"]]) for j in higher):
+                    logger.warning("Skill '%s' at %s is shadowed by a higher-precedence copy "
+                                   "(project > local > create_dir > external_dirs)", name, e["path"])
         elif winner[name] == i:
             e.update(status="unique", load_name=name)
         else:
diff --git a/tests/tools/test_skills_tool.py b/tests/tools/test_skills_tool.py
index b89a601caf..605ca2da73 100644
--- a/tests/tools/test_skills_tool.py
+++ b/tests/tools/test_skills_tool.py
@@ -909,7 +909,7 @@ class TestSkillViewCollisionDetection:
                 patch.object(skill_utils, "iter_project_skill_files",
                              lambda d: skill_utils.iter_skill_index_files(d, "SKILL.md")):
             skills_tool_module._SKILLS_CACHE.clear()
-            skill_utils._WARNED_SHADOWS.clear()
+            skill_utils._SHADOW_CHECKED.clear()
             result = json.loads(skill_view(lookup))
             listed = json.loads(skills_list())["skills"]