Skills grid: dedup a skill installed under a different source than the catalog lists it

library/Tiger/Agent/Skills.php

@@ -49,6 +49,7 @@ public static function installed()
                 'description' => $front['description'] ?? '',
                 'sourceLabel' => (string) ($meta['sourceLabel'] ?? ''),
                 'repo'        => (string) ($meta['repo'] ?? ''),
+                'path'        => (string) ($meta['path'] ?? ''),   // canonical skill location (repo + path) — for catalog dedup
                 'url'         => (string) ($meta['url'] ?? ''),
                 'active'      => in_array($key, $active, true),
                 'dir'         => $d,

modules/agent/services/Skills.php

@@ -67,25 +67,32 @@ public function datatable(array $params): void
         $search  = strtolower(trim((string) $dt['search']));
         $refresh = !empty($params['refresh']);
 
-        // Installed skills keyed by install key (key == safeKey(source__name), which installKey() mirrors).
+        // Installed skills, indexed two ways: by install key AND by canonical identity (repo|path). The
+        // canonical index is what dedups a skill installed via one source (e.g. a pasted URL → key
+        // `url__x`) against the SAME skill in a catalog (key `webtigers-skills__x`) — same repo+path, one row.
         $installed = [];
-        foreach (Tiger_Agent_Skills::installed() as $s) { $installed[$s['key']] = $s; }
+        $byLoc     = [];
+        foreach (Tiger_Agent_Skills::installed() as $s) {
+            $installed[$s['key']] = $s;
+            if ($s['repo'] !== '' && ($s['path'] ?? '') !== '') { $byLoc[self::_loc($s['repo'], $s['path'])] = $s; }
+        }
 
         $items = [];
-        $seen  = [];
+        $shown = [];   // install keys already emitted (matched to a catalog row) → skip in the tail
         // The browse catalog (per-source cached; only the sources scan hits the network, and only on refresh).
         foreach (Tiger_Skill_Index::all($refresh) as $e) {
-            $key  = Agent_Service_Skills::installKey($e);
-            $inst = $installed[$key] ?? null;
+            $catKey = Agent_Service_Skills::installKey($e);
+            $inst   = $installed[$catKey] ?? ($byLoc[self::_loc((string) $e['repo'], (string) $e['path'])] ?? null);
+            $key    = $inst !== null ? $inst['key'] : $catKey;   // actions target the ACTUAL installed dir
             $items[] = $this->_row($key, (string) $e['source'], $e['name'], $e['description'], $e['sourceLabel'],
                 $e['repo'], $e['ref'], $e['path'], $e['url'], $inst !== null, $inst !== null && !empty($inst['active']));
-            $seen[$key] = true;
+            if ($inst !== null) { $shown[$inst['key']] = true; }
         }
-        // Installed but not in any catalog (a pasted-URL install, or a source that's since delisted).
+        // Installed but not matched to any catalog entry (a pasted-URL install of a repo no source lists).
         foreach ($installed as $key => $s) {
-            if (isset($seen[$key])) { continue; }
+            if (isset($shown[$key])) { continue; }
             $items[] = $this->_row($key, '', $s['name'], $s['description'], $s['sourceLabel'],
-                (string) $s['repo'], '', '', (string) $s['url'], true, !empty($s['active']));
+                (string) $s['repo'], '', (string) ($s['path'] ?? ''), (string) $s['url'], true, !empty($s['active']));
         }
 
         if ($search !== '') {
@@ -107,6 +114,12 @@ public function datatable(array $params): void
         $this->_dtResponse($dt['draw'], $total, $total, $page);
     }
 
+    /** Canonical skill identity for dedup: repo + path, normalized (a skill is the same wherever installed from). */
+    private static function _loc($repo, $path): string
+    {
+        return strtolower(trim((string) $repo, '/') . '|' . trim((string) $path, '/'));
+    }
+
     /** One normalized grid row (a catalog entry and/or an installed skill). */
     private function _row($key, $source, $name, $desc, $sourceLabel, $repo, $ref, $path, $url, $installed, $active): array
     {

tests/Integration/Agent/SkillsTest.php

@@ -45,11 +45,24 @@ protected function tearDown(): void
     {
         foreach (['/SKILL.md', '/source.json'] as $f) { @unlink($this->skillDir . $f); }
         @rmdir($this->skillDir);
-        @rmdir(Tiger_Agent_Skills::dir());
+        \Tiger_Skill_Index::clearSources();
+        $this->_rrmdir(\Tiger_Agent_Skills::dir());
         foreach ($this->indexCaches as $f) { @unlink($f); }
+        @unlink(APPLICATION_ROOT . '/var/cache/skills/catalog-x.json');
         parent::tearDown();
     }
 
+    private function _rrmdir(string $dir): void
+    {
+        if ($dir === '' || !is_dir($dir)) { return; }
+        foreach (scandir($dir) ?: [] as $f) {
+            if ($f === '.' || $f === '..') { continue; }
+            $p = $dir . '/' . $f;
+            is_dir($p) ? $this->_rrmdir($p) : @unlink($p);
+        }
+        @rmdir($dir);
+    }
+
     private function call(string $action, array $params = []): object
     {
         return (new Agent_Service_Skills(['action' => $action] + $params))->getResponse();
@@ -162,4 +175,40 @@ public function datatable_is_denied_for_a_guest(): void
         $res = $this->call('datatable', ['draw' => 1, 'start' => 0, 'length' => 25]);
         $this->assertSame(0, (int) $res->result);
     }
+
+    #[Test]
+    public function datatable_dedups_a_skill_installed_under_a_different_source(): void
+    {
+        $this->loginAs('admin');
+
+        // Install a skill via one source id (mimics a pasted-URL install): key = url__widget, but its
+        // source.json records the canonical repo + path.
+        $dir = Tiger_Agent_Skills::dir() . '/url__widget';
+        @mkdir($dir, 0775, true);
+        file_put_contents($dir . '/SKILL.md', "---\nname: widget\ndescription: A widget skill.\n---\nBody.");
+        file_put_contents($dir . '/source.json', json_encode(['sourceLabel' => 'From github.com/acme/skills', 'repo' => 'acme/skills', 'path' => 'skills/widget']));
+
+        // The SAME skill in a catalog, under a DIFFERENT source id (→ installKey catalog-x__widget).
+        \Tiger_Skill_Index::registerSource(new DedupCatalogSource());
+
+        $rows   = $this->call('datatable', ['draw' => 1, 'start' => 0, 'length' => 25])->data['data'];
+        $widget = array_values(array_filter($rows, fn($r) => $r['name'] === 'widget'));
+
+        $this->assertCount(1, $widget, 'the same skill (same repo+path) appears ONCE, not once per source');
+        $this->assertTrue($widget[0]['installed'], 'it shows as installed, not Available');
+        $this->assertSame('url__widget', $widget[0]['key'], 'actions target the actual installed dir/key');
+
+        $this->_rrmdir($dir);
+    }
+}
+
+/** A catalog source that lists the same skill (repo+path) an installed one has, under a different id. */
+final class DedupCatalogSource extends \Tiger_Skill_Source
+{
+    public function id(): string    { return 'catalog-x'; }
+    public function label(): string { return 'Catalog X'; }
+    public function scan(): array
+    {
+        return [$this->entry('acme/skills', 'main', 'skills/widget', ['name' => 'widget', 'description' => 'A widget skill (from catalog).'])];
+    }
 }