diff --git a/library/Tiger/Agent/Skills.php b/library/Tiger/Agent/Skills.php index 40f51f7..9304785 100644 --- a/library/Tiger/Agent/Skills.php +++ b/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, diff --git a/modules/agent/services/Skills.php b/modules/agent/services/Skills.php index 103247c..e63bf88 100644 --- a/modules/agent/services/Skills.php +++ b/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 { diff --git a/tests/Integration/Agent/SkillsTest.php b/tests/Integration/Agent/SkillsTest.php index 2ff293d..432141d 100644 --- a/tests/Integration/Agent/SkillsTest.php +++ b/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).'])]; + } }