From d117451d7cbb3ec19b631d185e2fcef704345869 Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Sun, 21 Jun 2026 21:12:46 +0200 Subject: [PATCH 1/4] [Mate] Add skills:install to install extension skills for coding agents Agent Skills (SKILL.md) shipped by Mate extensions are now installed onto the filesystem where coding agents read them, as a polyfill until skills can be served over MCP directly. Extensions declare a skills directory via the new extra.ai-mate.skills key. The skills:install command installs them into .agents/skills (read by Codex, OpenCode and Copilot) and symlinks them into .claude/skills for Claude Code. Skills are copied by default (editable in the project) or symlinked to the vendor package with --mode=symlink. discover runs the install automatically, so skills appear after composer require with no extra step. Individual skills can be disabled with --skip, persisted in mate/skills.php. Ships a built-in system-information skill on the Mate package itself. --- demo/.gitignore | 5 + docs/components/mate.rst | 31 +++ docs/components/mate/creating-extensions.rst | 34 ++++ src/mate/AGENTS.md | 10 +- src/mate/CHANGELOG.md | 5 + src/mate/composer.json | 6 +- src/mate/skills/system-information/SKILL.md | 72 +++++++ src/mate/src/App.php | 2 + src/mate/src/Command/DiscoverCommand.php | 35 ++++ src/mate/src/Command/SkillsInstallCommand.php | 108 ++++++++++ .../Discovery/ComposerExtensionDiscovery.php | 80 +++++++- src/mate/src/Service/SkillsInstaller.php | 187 ++++++++++++++++++ src/mate/src/default.config.php | 13 ++ .../AgentInstructionsMaterializerTest.php | 3 +- .../Command/DebugCapabilitiesCommandTest.php | 44 ++--- .../Command/DebugExtensionsCommandTest.php | 43 ++-- .../tests/Command/DiscoverCommandTest.php | 84 +++++++- .../Command/SkillsInstallCommandTest.php | 145 ++++++++++++++ .../ComposerExtensionDiscoveryTest.php | 47 +++++ .../vendor/composer/installed.json | 14 ++ .../skills/demo-skill/SKILL.md | 8 + .../vendor/package-with-skills/src/.gitkeep | 0 .../tests/Service/SkillsInstallerTest.php | 182 +++++++++++++++++ 23 files changed, 1105 insertions(+), 53 deletions(-) create mode 100644 src/mate/skills/system-information/SKILL.md create mode 100644 src/mate/src/Command/SkillsInstallCommand.php create mode 100644 src/mate/src/Service/SkillsInstaller.php create mode 100644 src/mate/tests/Command/SkillsInstallCommandTest.php create mode 100644 src/mate/tests/Discovery/Fixtures/with-skills/vendor/composer/installed.json create mode 100644 src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/skills/demo-skill/SKILL.md create mode 100644 src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/src/.gitkeep create mode 100644 src/mate/tests/Service/SkillsInstallerTest.php diff --git a/demo/.gitignore b/demo/.gitignore index cb9fd759ba..2d836ed42f 100644 --- a/demo/.gitignore +++ b/demo/.gitignore @@ -21,4 +21,9 @@ phpstan.neon .phpunit.cache ###< phpunit/phpunit ### +###> symfony/ai-mate ### +/.agents/skills/ +/.claude/skills/ +###< symfony/ai-mate ### + pgdata diff --git a/docs/components/mate.rst b/docs/components/mate.rst index 45abc0de79..a4242300dd 100644 --- a/docs/components/mate.rst +++ b/docs/components/mate.rst @@ -413,6 +413,31 @@ The core package provides basic system information tools: * ``server-info`` - Get PHP runtime environment details: version, OS, OS family, and loaded extensions +Skills +------ + +`Agent Skills `_ are ``SKILL.md`` files that give your coding agent +structured, multi-step "how-to" knowledge for a task. Extensions can ship skills alongside their MCP tools, and Mate +installs them onto the filesystem where coding agents read them — a polyfill until skills can be +served over MCP directly. + +You usually do not run anything: ``mate discover`` (which also runs automatically after +``composer require``) installs the skills of every enabled extension. To sync them manually, use +``mate skills:install``. + +Each skill is installed under a ``mate-`` prefixed directory name (e.g. ``mate-demo-skill``) to +avoid clashing with skills you maintain from other sources, in two locations: + +* ``.agents/skills/`` is the source of truth, read directly by Codex, OpenCode and GitHub Copilot. +* ``.claude/skills/`` is symlinked to ``.agents/skills/`` for Claude Code, which only reads its own + directory. + +Skills are symlinked into ``.agents/skills/`` so they auto-update on ``composer update`` (the link +points into the gitignored ``vendor/`` directory, so this requires symlink privileges on Windows). + +The core package itself ships a ``system-information`` skill describing how to inspect the PHP +runtime and installed package versions via the ``server-info`` tool. + Commands -------- @@ -426,6 +451,12 @@ Commands - Generate or update ``mate/extensions.php`` with discovered extensions - Preserve existing enabled/disabled states for known extensions - Default new extensions to enabled + - Install Agent Skills shipped by enabled extensions (see `Skills`_) + +``mate skills:install`` + Install the Agent Skills shipped by your enabled extensions so your coding agent can use + them. This runs automatically as part of ``mate discover``; use it for an explicit re-sync. + See `Skills`_. ``mate serve`` Start the MCP server with stdio transport. diff --git a/docs/components/mate/creating-extensions.rst b/docs/components/mate/creating-extensions.rst index 22c0c67a3c..54125087a6 100644 --- a/docs/components/mate/creating-extensions.rst +++ b/docs/components/mate/creating-extensions.rst @@ -165,6 +165,40 @@ Example configuration: } } +Skills +~~~~~~ + +``extra.ai-mate.skills`` (optional) + +- List of directories holding `Agent Skills `_, relative to package root + (a single string is also accepted) +- Each immediate subdirectory is one skill and must contain a ``SKILL.md`` file +- Conventionally a single ``skills`` directory +- When a project runs ``mate discover`` (or ``mate skills:install``), each skill directory is + installed into the project's ``.agents/skills/`` and symlinked into ``.claude/skills/`` so coding + agents can use it + +Example configuration: + +.. code-block:: json + + { + "extra": { + "ai-mate": { + "scan-dirs": ["src"], + "skills": ["skills"] + } + } + } + +The directory layout for the example above:: + + skills/ + └── my-skill/ + ├── SKILL.md + └── references/ + └── details.md + Extension Discovery Opt-Out ~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/mate/AGENTS.md b/src/mate/AGENTS.md index 4958ee621d..cfdce97c2d 100644 --- a/src/mate/AGENTS.md +++ b/src/mate/AGENTS.md @@ -43,7 +43,8 @@ cd ../../.. && vendor/bin/php-cs-fixer fix src/mate/ ### Running the Server ```bash bin/mate init # Initialize configuration -bin/mate discover # Discover extensions +bin/mate discover # Discover extensions (also installs extension skills) +bin/mate skills:install # Install extension skills bin/mate serve # Start MCP server bin/mate clear-cache # Clear cache @@ -67,6 +68,13 @@ Running `bin/mate discover` generates `mate/AGENT_INSTRUCTIONS.md` with extensio - `mate/.env`: Environment variables for mate configuration - `mate/src/`: Directory for user-defined MCP tools +## Skills + +Extensions ship Agent Skills (`SKILL.md`) via the `extra.ai-mate.skills` directory key. `skills:install` +(run automatically by `discover`) symlinks each skill under a `mate-` prefixed directory into `.agents/skills/` +(read by Codex/OpenCode/Copilot) and into `.claude/skills/` for Claude Code, so they auto-update with the +package. The core package ships a built-in `system-information` skill. + ### Extension Exclusion By default, `mate init` sets `extension: false` in `composer.json` so vendor packages using Mate for internal tooling are not detected as user-facing extensions. To make a package a discoverable Mate extension, set `"extension": true` or remove the field entirely. diff --git a/src/mate/CHANGELOG.md b/src/mate/CHANGELOG.md index 10e8491c59..d6e7ed7c1b 100644 --- a/src/mate/CHANGELOG.md +++ b/src/mate/CHANGELOG.md @@ -1,6 +1,11 @@ CHANGELOG ========= +0.11 +---- + + * Add `skills:install` command and `extra.ai-mate.skills` config key to install Agent Skills shipped by extensions into `.agents/skills` and `.claude/skills` for coding agents + 0.9 --- diff --git a/src/mate/composer.json b/src/mate/composer.json index c8d4680c51..7cd9788642 100644 --- a/src/mate/composer.json +++ b/src/mate/composer.json @@ -37,6 +37,7 @@ "symfony/console": "^5.4|^6.4|^7.3|^8.0", "symfony/dependency-injection": "^5.4|^6.4|^7.3|^8.0", "symfony/ai-mate-composer-plugin": "^0.10", + "symfony/filesystem": "^5.4|^6.4|^7.3|^8.0", "symfony/finder": "^5.4|^6.4|^7.3|^8.0" }, "require-dev": { @@ -75,7 +76,10 @@ "scan-dirs": [ "src/Capability" ], - "instructions": "INSTRUCTIONS.md" + "instructions": "INSTRUCTIONS.md", + "skills": [ + "skills" + ] } } } diff --git a/src/mate/skills/system-information/SKILL.md b/src/mate/skills/system-information/SKILL.md new file mode 100644 index 0000000000..cccec27d07 --- /dev/null +++ b/src/mate/skills/system-information/SKILL.md @@ -0,0 +1,72 @@ +--- +name: system-information +description: >- + Inspect the runtime and dependency context of the PHP application Mate is attached to — + PHP version, OS, and loaded extensions via the server-info tool, plus installed package + versions from composer.lock — when diagnosing environment- or version-specific behavior + such as a missing extension or a version mismatch (an API that does not exist in the + release that is actually installed). +--- + +# System Information + +Use this skill when a problem might depend on the **environment or the installed +dependency versions** rather than the code: a missing or misconfigured extension, +behavior that only reproduces on a specific PHP/OS version, or a bug that looks like a +version mismatch — a method, argument, or class that does not exist in the release that +is actually installed. + +## Capabilities + +| Need | Use | +|---|---| +| PHP version, OS, OS family, loaded extensions | `server-info` MCP tool (one call) | +| Installed version of a package | `composer show ` | +| Installed versions of a family of packages | `composer show '/*'` | +| Authoritative record of every installed version | `composer.lock` (`packages` / `packages-dev`) | + +`server-info` reports the runtime only — it does **not** return package versions. Use +Composer for those. + +## Runtime checks + +1. **Start with the `server-info` tool** for PHP version, OS, OS family, and loaded + extensions — prefer it over `php -v`, `php -m`, or `uname`. +2. **Confirm a required extension is loaded** before blaming the code. If it is absent + from `server-info`, the fix is an environment/configuration change, not a code change. + +## Resolving package versions + +`composer.json` declares *constraints* (what is allowed); `composer.lock` records what is +*actually installed*. For "what is really running", trust the lock, not the constraint. + +- **One package:** `composer show symfony/console` → installed version plus details. +- **A family:** `composer show 'symfony/ai-*'` → every matching package and its version. +- **Machine-readable:** `composer show --format=json symfony/console`. +- **No Composer binary, or you want the raw record:** look the package up by `name` under + `packages` in `composer.lock` and read its `version` field — do not load the whole lock + file into context. + +Monorepo / path-repo note: locally linked packages (for example symlinked `symfony/ai-*` +in this repository) report `dev-main` or a path reference instead of a semver tag — that +is the working copy, not a released version. + +## Diagnosing a version mismatch + +1. Identify the package behind the failing API — the namespace usually maps to it + (`Symfony\Component\Console\…` → `symfony/console`). +2. Read the **installed** version with `composer show ` (or from `composer.lock`). +3. Compare it against the version the code expects: a method or argument added in a newer + minor, or removed/renamed in a newer major, is the classic signature. +4. Report the gap precisely — name the package, the installed version, and the required + one: *"symfony/console is 6.4 (installed), but the code calls a 7.1 API."* + +## Rules + +- Reach for `server-info` and `composer show` first; they are cheaper and more structured + than reading `composer.lock` or `php -m` wholesale. +- Report only the *one or two* facts that change the diagnosis (e.g. "ext-intl is not + loaded", "symfony/console is 6.4, the code uses a 7.1 API") — never the full extension + dump or the entire lock file. +- Never run state-changing commands. `composer show` is read-only; `composer require`, + `composer update`, and `composer install` are not. This skill is read-only inspection. diff --git a/src/mate/src/App.php b/src/mate/src/App.php index 0427ceb991..02e49c5a49 100644 --- a/src/mate/src/App.php +++ b/src/mate/src/App.php @@ -20,6 +20,7 @@ use Symfony\AI\Mate\Command\InitCommand; use Symfony\AI\Mate\Command\ResourcesReadCommand; use Symfony\AI\Mate\Command\ServeCommand; +use Symfony\AI\Mate\Command\SkillsInstallCommand; use Symfony\AI\Mate\Command\StopCommand; use Symfony\AI\Mate\Command\ToolsCallCommand; use Symfony\AI\Mate\Command\ToolsInspectCommand; @@ -54,6 +55,7 @@ public static function build(ContainerInterface $container): Application ToolsInspectCommand::class, ToolsCallCommand::class, ResourcesReadCommand::class, + SkillsInstallCommand::class, ]; foreach ($commands as $commandClass) { diff --git a/src/mate/src/Command/DiscoverCommand.php b/src/mate/src/Command/DiscoverCommand.php index d245c3cf12..a200e2d534 100644 --- a/src/mate/src/Command/DiscoverCommand.php +++ b/src/mate/src/Command/DiscoverCommand.php @@ -14,6 +14,7 @@ use Symfony\AI\Mate\Agent\AgentInstructionsMaterializer; use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; use Symfony\AI\Mate\Service\ExtensionConfigSynchronizer; +use Symfony\AI\Mate\Service\SkillsInstaller; use Symfony\Component\Console\Attribute\AsCommand; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputInterface; @@ -28,6 +29,9 @@ * and generates/updates mate/extensions.php with discovered extensions. * Also refreshes AGENT instruction artifacts for coding agents. * + * @phpstan-import-type ExtensionData from ComposerExtensionDiscovery + * @phpstan-import-type InstallResult from SkillsInstaller + * * @author Johannes Wachter * @author Tobias Nyholm */ @@ -38,6 +42,7 @@ public function __construct( private ComposerExtensionDiscovery $extensionDiscovery, private ExtensionConfigSynchronizer $extensionConfigSynchronizer, private AgentInstructionsMaterializer $instructionsMaterializer, + private SkillsInstaller $skillsInstaller, ) { parent::__construct(self::getDefaultName()); } @@ -118,6 +123,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int $materializationResult = $this->instructionsMaterializer->materializeForExtensions($enabledExtensionsForInstructions); + $skillResult = $this->syncSkills($enabledExtensionsForInstructions); + if ($composerMode) { $this->displayComposerSummary($io, $count, $newPackages, $removedPackages); @@ -152,6 +159,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int } $this->displayInstructionsStatus($io, $materializationResult); + $this->displaySkillStatus($io, $skillResult); $io->comment([ 'Next steps:', @@ -161,6 +169,33 @@ protected function execute(InputInterface $input, OutputInterface $output): int return Command::SUCCESS; } + /** + * Install skills shipped by enabled extensions into the agent skill directories. + * + * Only new/missing skills are installed here; existing entries are left untouched. + * Extensions without skills are filtered out by the installer. + * + * @param array $enabledExtensions enabled extensions plus the root project ("_custom") + * + * @return InstallResult + */ + private function syncSkills(array $enabledExtensions): array + { + return $this->skillsInstaller->install($enabledExtensions); + } + + /** + * @param InstallResult $skillResult + */ + private function displaySkillStatus(SymfonyStyle $io, array $skillResult): void + { + if ([] === $skillResult['installed']) { + return; + } + + $io->text(\sprintf('Installed %d skill(s): %s.', \count($skillResult['installed']), implode(', ', $skillResult['installed']))); + } + /** * @param string[] $newPackages * @param string[] $removedPackages diff --git a/src/mate/src/Command/SkillsInstallCommand.php b/src/mate/src/Command/SkillsInstallCommand.php new file mode 100644 index 0000000000..efa2c5864c --- /dev/null +++ b/src/mate/src/Command/SkillsInstallCommand.php @@ -0,0 +1,108 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\AI\Mate\Command; + +use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; +use Symfony\AI\Mate\Service\SkillsInstaller; +use Symfony\Component\Console\Attribute\AsCommand; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Style\SymfonyStyle; + +/** + * Install extension-provided Agent Skills onto the filesystem so coding agents can use them. + * + * @phpstan-import-type ExtensionData from ComposerExtensionDiscovery + * + * @author Johannes Wachter + */ +#[AsCommand('skills:install', 'Install extension skills so coding agents can use them')] +class SkillsInstallCommand extends Command +{ + /** + * @param string[] $enabledExtensions + */ + public function __construct( + private array $enabledExtensions, + private ComposerExtensionDiscovery $extensionDiscovery, + private SkillsInstaller $installer, + ) { + parent::__construct(self::getDefaultName()); + } + + public static function getDefaultName(): string + { + return 'skills:install'; + } + + public static function getDefaultDescription(): string + { + return 'Install extension skills so coding agents can use them'; + } + + protected function configure(): void + { + $this + ->setHelp( + <<<'HELP' +The %command.name% command installs Agent Skills shipped by your installed +Mate extensions so your coding agent (Claude Code, Codex, OpenCode, Copilot, …) can use them. + +Each skill is symlinked under a mate- prefixed directory into a shared +location (.agents/skills, read by Codex, OpenCode and Copilot) and into +.claude/skills for Claude Code, so they auto-update with the package (the +link points into the gitignored vendor/ and needs symlink privileges on Windows). + +This runs automatically as part of mate discover; use this command for an explicit +re-sync. + + # Install all skills from enabled extensions + %command.full_name% +HELP + ); + } + + protected function execute(InputInterface $input, OutputInterface $output): int + { + $io = new SymfonyStyle($input, $output); + $io->title('Mate Skills'); + + $result = $this->installer->install($this->collectExtensions()); + + if ([] === $result['installed']) { + $io->note('No new skills to install.'); + + return Command::SUCCESS; + } + + $io->success(\sprintf('Installed %d skill(s): %s', \count($result['installed']), implode(', ', $result['installed']))); + + return Command::SUCCESS; + } + + /** + * @return array + */ + private function collectExtensions(): array + { + $extensions = [ + '_custom' => $this->extensionDiscovery->discoverRootProject(), + ]; + + foreach ($this->extensionDiscovery->discover($this->enabledExtensions) as $packageName => $data) { + $extensions[$packageName] = $data; + } + + return $extensions; + } +} diff --git a/src/mate/src/Discovery/ComposerExtensionDiscovery.php b/src/mate/src/Discovery/ComposerExtensionDiscovery.php index 4b3c400ee4..0906fbc793 100644 --- a/src/mate/src/Discovery/ComposerExtensionDiscovery.php +++ b/src/mate/src/Discovery/ComposerExtensionDiscovery.php @@ -31,6 +31,7 @@ * dirs: string[], * includes: string[], * instructions?: string, + * skills: string[], * } * * @author Johannes Wachter @@ -82,11 +83,13 @@ public function discover(array|false $includeFilter = false): array $scanDirs = $this->extractScanDirs($package, $packageName); $includeFiles = $this->extractIncludeFiles($package, $packageName); $agentInstructions = $this->extractInstructions($package, $packageName); + $skillsDirs = $this->extractSkillsDirs($package, $packageName); - if ([] !== $scanDirs || [] !== $includeFiles || null !== $agentInstructions) { + if ([] !== $scanDirs || [] !== $includeFiles || null !== $agentInstructions || [] !== $skillsDirs) { $extensionData = [ 'dirs' => $scanDirs, 'includes' => $includeFiles, + 'skills' => $skillsDirs, ]; if (null !== $agentInstructions) { @@ -110,6 +113,7 @@ public function discoverRootProject(): array return [ 'dirs' => [], 'includes' => [], + 'skills' => [], ]; } @@ -123,6 +127,7 @@ public function discoverRootProject(): array return [ 'dirs' => [], 'includes' => [], + 'skills' => [], ]; } @@ -131,12 +136,27 @@ public function discoverRootProject(): array return [ 'dirs' => [], 'includes' => [], + 'skills' => [], ]; } + $skillsDirs = []; + foreach ($this->extractAiMateConfig($rootComposer, 'skills') as $dir) { + // TODO: replace the manual ".." check with PathGuard::hasTraversal() once #2245 is merged. + if ('' === trim($dir) || str_contains($dir, '..')) { + continue; + } + + $relativePath = ltrim($dir, '/'); + if (is_dir($this->rootDir.'/'.$relativePath)) { + $skillsDirs[] = $relativePath; + } + } + $result = [ 'dirs' => array_values($this->extractAiMateConfig($rootComposer, 'scan-dirs')), 'includes' => array_values($this->extractAiMateConfig($rootComposer, 'includes')), + 'skills' => $skillsDirs, ]; $agentInstructions = $this->extractAiMateConfigString($rootComposer, 'instructions'); @@ -400,6 +420,64 @@ private function extractInstructions(array $package, string $packageName): ?stri return $agentInstructions; } + /** + * Extract skills directories from package extra config. + * + * Uses "skills" from extra.ai-mate config, e.g.: + * "extra": { "ai-mate": { "skills": ["skills"] } } + * + * Each directory holds one subdirectory per skill, each with a SKILL.md file. + * A single string is accepted and treated as a one-element list. + * + * @param array{ + * name: string, + * extra: array, + * } $package + * + * @return string[] list of project-relative paths (vendor//) + */ + private function extractSkillsDirs(array $package, string $packageName): array + { + $aiMateConfig = $package['extra']['ai-mate'] ?? null; + if (null === $aiMateConfig || !\is_array($aiMateConfig)) { + return []; + } + + $skillsDirs = $aiMateConfig['skills'] ?? []; + + // Support single directory as string + if (\is_string($skillsDirs)) { + $skillsDirs = [$skillsDirs]; + } + + if (!\is_array($skillsDirs)) { + $this->logger->warning('Invalid skills in ai-mate config', ['package' => $packageName]); + + return []; + } + + $validDirs = []; + foreach ($skillsDirs as $dir) { + // TODO: replace the manual ".." check with PathGuard::hasTraversal() once #2245 is merged. + if (!\is_string($dir) || '' === trim($dir) || str_contains($dir, '..')) { + continue; + } + + $relativePath = 'vendor/'.$packageName.'/'.ltrim($dir, '/'); + if (!is_dir($this->rootDir.'/'.$relativePath)) { + $this->logger->warning('Skills directory does not exist', [ + 'package' => $packageName, + 'directory' => $relativePath, + ]); + continue; + } + + $validDirs[] = $relativePath; + } + + return $validDirs; + } + /** * @param array $composer * diff --git a/src/mate/src/Service/SkillsInstaller.php b/src/mate/src/Service/SkillsInstaller.php new file mode 100644 index 0000000000..79ac202f8a --- /dev/null +++ b/src/mate/src/Service/SkillsInstaller.php @@ -0,0 +1,187 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\AI\Mate\Service; + +use Psr\Log\LoggerInterface; +use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; +use Symfony\Component\Filesystem\Filesystem; + +/** + * Installs extension-provided Agent Skills (SKILL.md folders) onto the filesystem + * so coding agents can use them, until MCP can serve skills directly. + * + * Skills land in a single source-of-truth directory (default .agents/skills, read by + * Codex/OpenCode/Copilot) and are symlinked into per-agent mirror directories + * (default .claude/skills for Claude Code, which does not read .agents/skills). + * + * @phpstan-import-type ExtensionData from ComposerExtensionDiscovery + * + * @phpstan-type InstallResult array{ + * installed: string[], + * } + * + * @author Johannes Wachter + */ +final class SkillsInstaller +{ + /** + * @param array $skillMirrors map of agent label to project-relative mirror dir (always symlinked) + */ + public function __construct( + private string $rootDir, + private LoggerInterface $logger, + private Filesystem $filesystem, + private string $skillsDir, + private array $skillMirrors, + ) { + } + + /** + * Installed skills are placed under a "mate-" prefixed directory name (e.g. "mate-demo-skill") + * to avoid clashing with skills the user maintains from other sources. + * + * @param array $extensions map of package name to extension data; entries without skills are skipped + * + * @return InstallResult + */ + public function install(array $extensions): array + { + $result = [ + 'installed' => [], + ]; + + $sourceRoot = $this->rootDir.'/'.trim($this->skillsDir, '/'); + $installedDirs = []; + + foreach ($extensions as $data) { + if ([] === $data['skills']) { + continue; + } + + foreach ($data['skills'] as $skillsRelDir) { + $skillsPath = $this->rootDir.'/'.trim($skillsRelDir, '/'); + foreach ($this->findSkills($skillsPath) as $name => $skillSource) { + $dirName = 'mate-'.$name; + + // First writer wins on cross-extension name collisions. + if (\in_array($dirName, $installedDirs, true)) { + $this->logger->warning('Duplicate skill name; keeping the first occurrence', ['skill' => $name]); + continue; + } + + $target = $sourceRoot.'/'.$dirName; + + if (file_exists($target) || is_link($target)) { + // Already installed; still track it so the mirror is refreshed. + $installedDirs[] = $dirName; + continue; + } + + $this->placeSkill($skillSource, $target); + $result['installed'][] = $dirName; + $installedDirs[] = $dirName; + } + } + } + + foreach ($this->skillMirrors as $mirrorDir) { + $this->refreshMirror($installedDirs, $mirrorDir); + } + + return $result; + } + + /** + * Find immediate subdirectories that contain a SKILL.md file. + * + * @return array map of skill name to absolute source directory + */ + private function findSkills(string $skillsPath): array + { + if (!is_dir($skillsPath)) { + return []; + } + + $entries = scandir($skillsPath); + if (false === $entries) { + return []; + } + + $skills = []; + foreach ($entries as $entry) { + // "." and ".." are scandir's own entries. Skill names are single path segments + // so they cannot traverse, but once #2245 (PathGuard) is merged route them through + // PathGuard::hasTraversal() to keep traversal handling consistent with discovery. + if ('.' === $entry || '..' === $entry) { + continue; + } + + $skillDir = $skillsPath.'/'.$entry; + if (!is_dir($skillDir) || !is_file($skillDir.'/SKILL.md')) { + continue; + } + + $skills[$entry] = $skillDir; + } + + return $skills; + } + + private function placeSkill(string $source, string $target): void + { + $parent = \dirname($target); + if (!is_dir($parent)) { + $this->filesystem->mkdir($parent); + } + + $this->filesystem->symlink(rtrim($this->filesystem->makePathRelative($source, $parent), '/'), $target); + } + + /** + * Create/refresh always-symlink mirror entries pointing at the source-of-truth skills. + * + * @param string[] $skillDirs prefixed skill directory names (e.g. "mate-demo-skill") + */ + private function refreshMirror(array $skillDirs, string $mirrorDir): void + { + $sourceRoot = $this->rootDir.'/'.trim($this->skillsDir, '/'); + $mirrorRoot = $this->rootDir.'/'.trim($mirrorDir, '/'); + + foreach ($skillDirs as $dirName) { + $sourceSkill = $sourceRoot.'/'.$dirName; + if (!is_dir($sourceSkill)) { + continue; + } + + $mirrorSkill = $mirrorRoot.'/'.$dirName; + $relativeTarget = rtrim($this->filesystem->makePathRelative($sourceSkill, $mirrorRoot), '/'); + + if (is_link($mirrorSkill)) { + if (readlink($mirrorSkill) === $relativeTarget) { + continue; + } + + // Repair a wrong or dangling link; the mirror is fully managed by Mate. + $this->filesystem->remove($mirrorSkill); + } elseif (file_exists($mirrorSkill)) { + // A real file or directory we do not own — leave it untouched. + continue; + } + + if (!is_dir($mirrorRoot)) { + $this->filesystem->mkdir($mirrorRoot); + } + + $this->filesystem->symlink($relativeTarget, $mirrorSkill); + } + } +} diff --git a/src/mate/src/default.config.php b/src/mate/src/default.config.php index 4cdde0b632..a51866547a 100644 --- a/src/mate/src/default.config.php +++ b/src/mate/src/default.config.php @@ -21,6 +21,7 @@ use Symfony\AI\Mate\Command\InitCommand; use Symfony\AI\Mate\Command\ResourcesReadCommand; use Symfony\AI\Mate\Command\ServeCommand; +use Symfony\AI\Mate\Command\SkillsInstallCommand; use Symfony\AI\Mate\Command\StopCommand; use Symfony\AI\Mate\Command\ToolsCallCommand; use Symfony\AI\Mate\Command\ToolsInspectCommand; @@ -31,8 +32,10 @@ use Symfony\AI\Mate\Service\ExtensionConfigSynchronizer; use Symfony\AI\Mate\Service\Logger; use Symfony\AI\Mate\Service\RegistryProvider; +use Symfony\AI\Mate\Service\SkillsInstaller; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; +use Symfony\Component\Filesystem\Filesystem; return static function (ContainerConfigurator $container): void { $debugLogFile = $_SERVER['MATE_DEBUG_LOG_FILE'] ?? 'dev.log'; @@ -47,6 +50,8 @@ ->set('mate.cache_dir', sys_get_temp_dir().'/mate') ->set('mate.env_file', null) ->set('mate.disabled_features', []) + ->set('mate.skills_dir', '.agents/skills') + ->set('mate.skill_mirrors', ['claude' => '.claude/skills']) ->set('mate.debug_log_file', $debugLogFile) ->set('mate.debug_file_enabled', $debugFileEnabled) ->set('mate.debug_enabled', $debugEnabled) @@ -64,6 +69,8 @@ ->bind('$disabledFeatures', '%mate.disabled_features%') ->bind('$enabledExtensions', '%mate.enabled_extensions%') ->bind('$mcpProtocolVersion', '%mate.mcp_protocol_version%') + ->bind('$skillsDir', '%mate.skills_dir%') + ->bind('$skillMirrors', '%mate.skill_mirrors%') ->set('_build.logger', Logger::class) ->private() // To be removed when we compile @@ -97,6 +104,9 @@ ->set(AgentInstructionsMaterializer::class) ->set(ExtensionConfigSynchronizer::class) + ->set(Filesystem::class) + ->set(SkillsInstaller::class) + // Register all commands ->set(InitCommand::class) ->public() @@ -130,5 +140,8 @@ ->set(ResourcesReadCommand::class) ->public() + + ->set(SkillsInstallCommand::class) + ->public() ; }; diff --git a/src/mate/tests/Agent/AgentInstructionsMaterializerTest.php b/src/mate/tests/Agent/AgentInstructionsMaterializerTest.php index ba936bef04..2666e835a7 100644 --- a/src/mate/tests/Agent/AgentInstructionsMaterializerTest.php +++ b/src/mate/tests/Agent/AgentInstructionsMaterializerTest.php @@ -47,10 +47,11 @@ public function testMaterializeForExtensionsWritesInstructionArtifacts() $materializer = new AgentInstructionsMaterializer($this->tempDir, $aggregator, $logger); $result = $materializer->materializeForExtensions([ - '_custom' => ['dirs' => [], 'includes' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], 'vendor/package-a' => [ 'dirs' => [], 'includes' => [], + 'skills' => [], 'instructions' => 'INSTRUCTIONS.md', ], ]); diff --git a/src/mate/tests/Command/DebugCapabilitiesCommandTest.php b/src/mate/tests/Command/DebugCapabilitiesCommandTest.php index 29722388e6..e2f77b87cb 100644 --- a/src/mate/tests/Command/DebugCapabilitiesCommandTest.php +++ b/src/mate/tests/Command/DebugCapabilitiesCommandTest.php @@ -38,8 +38,8 @@ public function testExecuteDisplaysCapabilities() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -57,8 +57,8 @@ public function testExecuteWithEmptyExtensions() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -75,8 +75,8 @@ public function testExecuteWithJsonFormat() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -102,8 +102,8 @@ public function testExecuteWithToonFormat() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -129,8 +129,8 @@ public function testExecuteWithExtensionFilter() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -149,8 +149,8 @@ public function testExecuteWithInvalidExtensionFilter() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -166,8 +166,8 @@ public function testExecuteWithTypeFilter() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -182,8 +182,8 @@ public function testExecuteWithInvalidTypeFilter() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -199,8 +199,8 @@ public function testExecuteWithCombinedFilters() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -218,8 +218,8 @@ public function testExecuteWithJsonFormatAndFilters() { $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $extensions = [ - 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['mate/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $extensions); @@ -242,8 +242,8 @@ public function testExecuteWithJsonFormatAndFilters() } /** - * @param array $extensions - * @param array> $disabledFeatures + * @param array $extensions + * @param array> $disabledFeatures */ private function createCommand(string $rootDir, array $extensions, array $disabledFeatures = []): DebugCapabilitiesCommand { diff --git a/src/mate/tests/Command/DebugExtensionsCommandTest.php b/src/mate/tests/Command/DebugExtensionsCommandTest.php index 32581e89f8..15fb7219e2 100644 --- a/src/mate/tests/Command/DebugExtensionsCommandTest.php +++ b/src/mate/tests/Command/DebugExtensionsCommandTest.php @@ -36,9 +36,9 @@ public function testExecuteDisplaysExtensions() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a', 'vendor/package-b']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - 'vendor/package-b' => ['dirs' => ['vendor/vendor/package-b/lib'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + 'vendor/package-b' => ['dirs' => ['vendor/vendor/package-b/lib'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -60,8 +60,8 @@ public function testExecuteShowsEnabledExtensionsOnly() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -81,8 +81,8 @@ public function testExecuteWithShowAllFlag() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -101,8 +101,8 @@ public function testExecuteWithJsonFormat() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -130,8 +130,8 @@ public function testExecuteWithToonFormat() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -159,8 +159,8 @@ public function testExecuteShowsExtensionDetails() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -178,7 +178,7 @@ public function testExecuteHandlesNoLoadedExtensions() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = []; $loadedExtensions = [ - '_custom' => ['dirs' => [], 'includes' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -196,8 +196,8 @@ public function testExecuteShowsLoadedStatus() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -214,8 +214,8 @@ public function testExecuteJsonFormatContainsExtensionMetadata() $rootDir = $this->fixturesDir.'/with-ai-mate-config'; $enabledExtensions = ['vendor/package-a']; $loadedExtensions = [ - 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => []], - '_custom' => ['dirs' => [], 'includes' => []], + 'vendor/package-a' => ['dirs' => ['vendor/vendor/package-a/src'], 'includes' => [], 'skills' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -243,8 +243,9 @@ public function testExecuteShowsIncludeFiles() 'vendor/package-with-includes' => [ 'dirs' => [], 'includes' => [$rootDir.'/vendor/vendor/package-with-includes/config/config.php'], + 'skills' => [], ], - '_custom' => ['dirs' => [], 'includes' => []], + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => []], ]; $command = $this->createCommand($rootDir, $enabledExtensions, $loadedExtensions); @@ -258,8 +259,8 @@ public function testExecuteShowsIncludeFiles() } /** - * @param array $enabledExtensions - * @param array $extensions + * @param array $enabledExtensions + * @param array $extensions */ private function createCommand(string $rootDir, array $enabledExtensions, array $extensions): DebugExtensionsCommand { diff --git a/src/mate/tests/Command/DiscoverCommandTest.php b/src/mate/tests/Command/DiscoverCommandTest.php index 355b6dca18..2208ffac6f 100644 --- a/src/mate/tests/Command/DiscoverCommandTest.php +++ b/src/mate/tests/Command/DiscoverCommandTest.php @@ -18,8 +18,10 @@ use Symfony\AI\Mate\Command\DiscoverCommand; use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; use Symfony\AI\Mate\Service\ExtensionConfigSynchronizer; +use Symfony\AI\Mate\Service\SkillsInstaller; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Tester\CommandTester; +use Symfony\Component\Filesystem\Filesystem; /** * @author Johannes Wachter @@ -46,7 +48,12 @@ public function testDiscoversExtensionsAndCreatesFile() $synchronizer = new ExtensionConfigSynchronizer($rootDir); $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); - $command = new DiscoverCommand($discoverer, $synchronizer, $materializer); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); $tester = new CommandTester($command); $tester->execute([]); @@ -102,7 +109,12 @@ public function testPreservesExistingEnabledState() $synchronizer = new ExtensionConfigSynchronizer($rootDir); $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); - $command = new DiscoverCommand($discoverer, $synchronizer, $materializer); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); $tester = new CommandTester($command); $tester->execute([]); @@ -139,7 +151,12 @@ public function testNewPackagesDefaultToEnabled() $synchronizer = new ExtensionConfigSynchronizer($rootDir); $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); - $command = new DiscoverCommand($discoverer, $synchronizer, $materializer); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); $tester = new CommandTester($command); $tester->execute([]); @@ -167,7 +184,12 @@ public function testIgnoreMissingFileExitsEarlyWhenExtensionsFileAbsent() $synchronizer = new ExtensionConfigSynchronizer($rootDir); $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); - $command = new DiscoverCommand($discoverer, $synchronizer, $materializer); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); $tester = new CommandTester($command); $tester->execute(['--ignore-missing-file' => true]); @@ -199,7 +221,12 @@ public function testIgnoreMissingFileRunsNormallyWhenExtensionsFileExists() $synchronizer = new ExtensionConfigSynchronizer($rootDir); $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); - $command = new DiscoverCommand($discoverer, $synchronizer, $materializer); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); $tester = new CommandTester($command); $tester->execute(['--ignore-missing-file' => true]); @@ -227,7 +254,12 @@ public function testDisplaysWarningWhenNoExtensionsFound() $synchronizer = new ExtensionConfigSynchronizer($rootDir); $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); - $command = new DiscoverCommand($discoverer, $synchronizer, $materializer); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); $tester = new CommandTester($command); $tester->execute([]); @@ -243,6 +275,42 @@ public function testDisplaysWarningWhenNoExtensionsFound() } } + public function testInstallsSkillsFromEnabledExtensions() + { + $tempDir = sys_get_temp_dir().'/mate-discover-test-'.uniqid(); + mkdir($tempDir, 0755, true); + + try { + $rootDir = $this->createConfiguration($this->fixturesDir.'/with-skills', $tempDir); + $logger = new NullLogger(); + $discoverer = new ComposerExtensionDiscovery($rootDir, $logger); + $synchronizer = new ExtensionConfigSynchronizer($rootDir); + $aggregator = new AgentInstructionsAggregator($rootDir, [], $logger); + $materializer = new AgentInstructionsMaterializer($rootDir, $aggregator, $logger); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($rootDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); + $tester = new CommandTester($command); + + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertFileExists($tempDir.'/.agents/skills/mate-demo-skill/SKILL.md'); + $this->assertTrue(is_link($tempDir.'/.claude/skills/mate-demo-skill')); + $this->assertStringContainsString('Installed 1 skill', $tester->getDisplay()); + + // Re-run must not overwrite (no force in discover): still a single install, nothing reported. + $rerun = new CommandTester($command); + $rerun->execute([]); + $this->assertStringNotContainsString('Installed 1 skill', $rerun->getDisplay()); + } finally { + $this->removeDirectory($tempDir); + } + } + private function createConfiguration(string $rootDir, string $tempDir): string { // Copy fixture to temp directory for testing @@ -286,6 +354,10 @@ private function removeDirectory(string $dir): void $files = array_diff(scandir($dir) ?: [], ['.', '..']); foreach ($files as $file) { $path = $dir.'/'.$file; + if (is_link($path)) { + unlink($path); + continue; + } is_dir($path) ? $this->removeDirectory($path) : unlink($path); } rmdir($dir); diff --git a/src/mate/tests/Command/SkillsInstallCommandTest.php b/src/mate/tests/Command/SkillsInstallCommandTest.php new file mode 100644 index 0000000000..afb80f1ccc --- /dev/null +++ b/src/mate/tests/Command/SkillsInstallCommandTest.php @@ -0,0 +1,145 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\AI\Mate\Tests\Command; + +use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; +use Symfony\AI\Mate\Command\SkillsInstallCommand; +use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; +use Symfony\AI\Mate\Service\SkillsInstaller; +use Symfony\Component\Console\Command\Command; +use Symfony\Component\Console\Tester\CommandTester; +use Symfony\Component\Filesystem\Filesystem; + +/** + * @author Johannes Wachter + */ +final class SkillsInstallCommandTest extends TestCase +{ + private Filesystem $filesystem; + + /** + * @var string[] + */ + private array $roots = []; + + protected function setUp(): void + { + $this->filesystem = new Filesystem(); + } + + protected function tearDown(): void + { + foreach ($this->roots as $root) { + $this->filesystem->remove($root); + } + } + + public function testInstallsSkills() + { + $root = $this->createRoot(); + $tester = new CommandTester($this->createCommand($root)); + + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringContainsString('Installed 1 skill', $tester->getDisplay()); + + $source = $root.'/.agents/skills/mate-demo-skill'; + $this->assertFileExists($source.'/SKILL.md'); + $this->assertTrue(is_link($source)); + $this->assertTrue(is_link($root.'/.claude/skills/mate-demo-skill')); + } + + public function testReRunSkipsExistingSkill() + { + $root = $this->createRoot(); + + (new CommandTester($this->createCommand($root)))->execute([]); + + $tester = new CommandTester($this->createCommand($root)); + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringContainsString('No new skills to install', $tester->getDisplay()); + } + + public function testReportsNothingWhenNoSkillsAvailable() + { + $root = sys_get_temp_dir().'/mate-skills-command-'.uniqid(); + $this->roots[] = $root; + $this->filesystem->mkdir($root.'/vendor/composer'); + file_put_contents($root.'/vendor/composer/installed.json', '{"packages": []}'); + + $tester = new CommandTester($this->createCommand($root)); + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringContainsString('No new skills to install', $tester->getDisplay()); + } + + public function testInstallsRootProjectSkill() + { + $root = sys_get_temp_dir().'/mate-skills-command-'.uniqid(); + $this->roots[] = $root; + + $this->filesystem->mkdir($root.'/vendor/composer'); + file_put_contents($root.'/vendor/composer/installed.json', '{"packages": []}'); + file_put_contents($root.'/composer.json', '{"extra": {"ai-mate": {"skills": ["skills"]}}}'); + $this->filesystem->mkdir($root.'/skills/root-skill'); + file_put_contents($root.'/skills/root-skill/SKILL.md', "---\nname: root-skill\ndescription: demo\n---\n"); + + $tester = new CommandTester($this->createCommand($root)); + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertFileExists($root.'/.agents/skills/mate-root-skill/SKILL.md'); + $this->assertTrue(is_link($root.'/.claude/skills/mate-root-skill')); + } + + private function createCommand(string $root): SkillsInstallCommand + { + $discovery = new ComposerExtensionDiscovery($root, new NullLogger()); + $installer = new SkillsInstaller($root, new NullLogger(), $this->filesystem, '.agents/skills', ['claude' => '.claude/skills']); + + return new SkillsInstallCommand( + ['vendor/package-with-skills'], + $discovery, + $installer, + ); + } + + private function createRoot(): string + { + $root = sys_get_temp_dir().'/mate-skills-command-'.uniqid(); + $this->roots[] = $root; + + $this->filesystem->mkdir($root.'/vendor/composer'); + file_put_contents($root.'/vendor/composer/installed.json', <<<'JSON' + { + "packages": [ + { + "name": "vendor/package-with-skills", + "type": "library", + "extra": { "ai-mate": { "skills": ["skills"] } } + } + ] + } + JSON); + + $skillDir = $root.'/vendor/vendor/package-with-skills/skills/demo-skill'; + $this->filesystem->mkdir($skillDir); + file_put_contents($skillDir.'/SKILL.md', "---\nname: demo-skill\ndescription: demo\n---\n"); + + return $root; + } +} diff --git a/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php b/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php index e869f212ca..8b7be01e44 100644 --- a/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php +++ b/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php @@ -14,6 +14,7 @@ use PHPUnit\Framework\TestCase; use Psr\Log\NullLogger; use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; +use Symfony\Component\Filesystem\Filesystem; /** * @author Johannes Wachter @@ -126,6 +127,52 @@ public function testExtractsIncludeFiles() $this->assertStringContainsString('config/config.php', $includes[0]); } + public function testExtractsSkillsDir() + { + $discovery = new ComposerExtensionDiscovery( + $this->fixturesDir.'/with-skills', + new NullLogger() + ); + + $extensions = $discovery->discover(); + + $this->assertArrayHasKey('vendor/package-with-skills', $extensions); + $this->assertArrayHasKey('skills', $extensions['vendor/package-with-skills']); + $this->assertSame(['vendor/vendor/package-with-skills/skills'], $extensions['vendor/package-with-skills']['skills']); + } + + public function testCoercesSingleSkillsStringToArray() + { + $rootDir = sys_get_temp_dir().'/mate-skills-string-'.uniqid(); + mkdir($rootDir.'/vendor/composer', 0755, true); + mkdir($rootDir.'/vendor/acme/pkg/skills', 0755, true); + file_put_contents($rootDir.'/vendor/composer/installed.json', '{"packages":[{"name":"acme/pkg","extra":{"ai-mate":{"skills":"skills"}}}]}'); + + try { + $discovery = new ComposerExtensionDiscovery($rootDir, new NullLogger()); + + $extensions = $discovery->discover(); + + $this->assertArrayHasKey('acme/pkg', $extensions); + $this->assertSame(['vendor/acme/pkg/skills'], $extensions['acme/pkg']['skills']); + } finally { + (new Filesystem())->remove($rootDir); + } + } + + public function testIgnoresMissingSkillsDir() + { + $discovery = new ComposerExtensionDiscovery( + $this->fixturesDir.'/with-includes', + new NullLogger() + ); + + $extensions = $discovery->discover(); + + $this->assertArrayHasKey('vendor/package-with-includes', $extensions); + $this->assertSame([], $extensions['vendor/package-with-includes']['skills']); + } + public function testHandlesMissingInstalledJson() { $discovery = new ComposerExtensionDiscovery( diff --git a/src/mate/tests/Discovery/Fixtures/with-skills/vendor/composer/installed.json b/src/mate/tests/Discovery/Fixtures/with-skills/vendor/composer/installed.json new file mode 100644 index 0000000000..a75ca790d4 --- /dev/null +++ b/src/mate/tests/Discovery/Fixtures/with-skills/vendor/composer/installed.json @@ -0,0 +1,14 @@ +{ + "packages": [ + { + "name": "vendor/package-with-skills", + "type": "library", + "extra": { + "ai-mate": { + "scan-dirs": ["src"], + "skills": ["skills"] + } + } + } + ] +} diff --git a/src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/skills/demo-skill/SKILL.md b/src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/skills/demo-skill/SKILL.md new file mode 100644 index 0000000000..1536860d69 --- /dev/null +++ b/src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/skills/demo-skill/SKILL.md @@ -0,0 +1,8 @@ +--- +name: demo-skill +description: A demo skill used by the discovery test fixtures. +--- + +# Demo Skill + +This is a fixture skill. diff --git a/src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/src/.gitkeep b/src/mate/tests/Discovery/Fixtures/with-skills/vendor/vendor/package-with-skills/src/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/mate/tests/Service/SkillsInstallerTest.php b/src/mate/tests/Service/SkillsInstallerTest.php new file mode 100644 index 0000000000..c39187b784 --- /dev/null +++ b/src/mate/tests/Service/SkillsInstallerTest.php @@ -0,0 +1,182 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\AI\Mate\Tests\Service; + +use PHPUnit\Framework\TestCase; +use Psr\Log\NullLogger; +use Symfony\AI\Mate\Service\SkillsInstaller; +use Symfony\Component\Filesystem\Filesystem; + +/** + * @author Johannes Wachter + */ +final class SkillsInstallerTest extends TestCase +{ + private string $rootDir; + private Filesystem $filesystem; + + protected function setUp(): void + { + $this->filesystem = new Filesystem(); + $this->rootDir = sys_get_temp_dir().'/mate-skills-installer-'.uniqid(); + + // Vendor skill tree: one valid skill (with a reference) and one junk dir without SKILL.md. + $skillsDir = $this->rootDir.'/vendor/vendor/pkg/skills'; + $this->filesystem->mkdir($skillsDir.'/demo-skill/references'); + file_put_contents($skillsDir.'/demo-skill/SKILL.md', "---\nname: demo-skill\ndescription: demo\n---\n"); + file_put_contents($skillsDir.'/demo-skill/references/collectors.md', "# ref\n"); + $this->filesystem->mkdir($skillsDir.'/not-a-skill'); + file_put_contents($skillsDir.'/not-a-skill/readme.md', "not a skill\n"); + } + + protected function tearDown(): void + { + $this->filesystem->remove($this->rootDir); + } + + public function testLinksSourceAndMirrorUnderPrefixedName() + { + $installer = $this->createInstaller(); + + $result = $installer->install($this->extensions()); + + $this->assertSame(['mate-demo-skill'], $result['installed']); + + $source = $this->rootDir.'/.agents/skills/mate-demo-skill'; + $this->assertTrue(is_link($source), 'Source-of-truth entry is symlinked into vendor'); + $this->assertFileExists($source.'/SKILL.md'); + $this->assertFileExists($source.'/references/collectors.md'); + + $this->assertDirectoryDoesNotExist($this->rootDir.'/.agents/skills/mate-not-a-skill'); + + $mirror = $this->rootDir.'/.claude/skills/mate-demo-skill'; + $this->assertTrue(is_link($mirror), 'Claude mirror is a symlink'); + $this->assertFileExists($mirror.'/SKILL.md'); + } + + public function testReRunSkipsExistingSkill() + { + $installer = $this->createInstaller(); + + $first = $installer->install($this->extensions()); + $this->assertSame(['mate-demo-skill'], $first['installed']); + + $second = $installer->install($this->extensions()); + $this->assertSame([], $second['installed'], 'Already-installed skills are not reinstalled'); + } + + public function testBrokenMirrorSymlinkIsRepaired() + { + $installer = $this->createInstaller(); + $installer->install($this->extensions()); + + // Corrupt the mirror so it dangles (points at a non-existent target). + $mirror = $this->rootDir.'/.claude/skills/mate-demo-skill'; + $this->filesystem->remove($mirror); + symlink('./does-not-exist', $mirror); + $this->assertFalse(file_exists($mirror), 'Mirror is dangling before repair'); + + // A plain re-run heals the dangling link. + $installer->install($this->extensions()); + + $this->assertTrue(is_link($mirror)); + $this->assertFileExists($mirror.'/SKILL.md', 'Mirror resolves to the installed skill again'); + } + + public function testCrossExtensionNameCollisionFirstWins() + { + $this->writeSkill('vendor/vendor/pkg-a/skills', 'shared', "from-a\n"); + $this->writeSkill('vendor/vendor/pkg-b/skills', 'shared', "from-b\n"); + + $installer = $this->createInstaller(); + $result = $installer->install([ + 'vendor/pkg-a' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/pkg-a/skills']], + 'vendor/pkg-b' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/pkg-b/skills']], + ]); + + $this->assertSame(['mate-shared'], $result['installed']); + $this->assertStringEqualsFile($this->rootDir.'/.agents/skills/mate-shared/SKILL.md', "from-a\n"); + } + + public function testMultipleSkillsFromOneExtension() + { + $this->writeSkill('vendor/vendor/multi/skills', 'one', "one\n"); + $this->writeSkill('vendor/vendor/multi/skills', 'two', "two\n"); + + $installer = $this->createInstaller(); + $result = $installer->install([ + 'vendor/multi' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/multi/skills']], + ]); + + sort($result['installed']); + $this->assertSame(['mate-one', 'mate-two'], $result['installed']); + $this->assertDirectoryExists($this->rootDir.'/.agents/skills/mate-one'); + $this->assertDirectoryExists($this->rootDir.'/.agents/skills/mate-two'); + $this->assertTrue(is_link($this->rootDir.'/.claude/skills/mate-one')); + $this->assertTrue(is_link($this->rootDir.'/.claude/skills/mate-two')); + } + + public function testMissingSkillsDirectoryIsIgnored() + { + $installer = $this->createInstaller(); + + $result = $installer->install([ + 'vendor/ghost' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/ghost/skills']], + ]); + + $this->assertSame([], $result['installed']); + $this->assertDirectoryDoesNotExist($this->rootDir.'/.agents/skills'); + } + + public function testRootProjectSkillsAreInstalled() + { + $this->writeSkill('skills', 'root-skill', "root\n"); + + $installer = $this->createInstaller(); + $result = $installer->install([ + '_custom' => ['dirs' => [], 'includes' => [], 'skills' => ['skills']], + ]); + + $this->assertSame(['mate-root-skill'], $result['installed']); + $this->assertFileExists($this->rootDir.'/.agents/skills/mate-root-skill/SKILL.md'); + $this->assertTrue(is_link($this->rootDir.'/.claude/skills/mate-root-skill')); + } + + /** + * @param array $mirrors + */ + private function createInstaller(array $mirrors = ['claude' => '.claude/skills']): SkillsInstaller + { + return new SkillsInstaller($this->rootDir, new NullLogger(), $this->filesystem, '.agents/skills', $mirrors); + } + + private function writeSkill(string $skillsRelDir, string $name, string $body): void + { + $dir = $this->rootDir.'/'.$skillsRelDir.'/'.$name; + $this->filesystem->mkdir($dir); + file_put_contents($dir.'/SKILL.md', $body); + } + + /** + * @return array + */ + private function extensions(): array + { + return [ + 'vendor/pkg' => [ + 'dirs' => [], + 'includes' => [], + 'skills' => ['vendor/vendor/pkg/skills'], + ], + ]; + } +} From 1f5c436684c465788aa2cee0a07777e4a4427ccd Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Sun, 28 Jun 2026 21:23:46 +0200 Subject: [PATCH 2/4] [Mate] Route skill paths through PathGuard::hasTraversal() --- src/mate/src/Discovery/ComposerExtensionDiscovery.php | 6 ++---- src/mate/src/Service/SkillsInstaller.php | 8 ++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/mate/src/Discovery/ComposerExtensionDiscovery.php b/src/mate/src/Discovery/ComposerExtensionDiscovery.php index 0906fbc793..e5a9b72263 100644 --- a/src/mate/src/Discovery/ComposerExtensionDiscovery.php +++ b/src/mate/src/Discovery/ComposerExtensionDiscovery.php @@ -142,8 +142,7 @@ public function discoverRootProject(): array $skillsDirs = []; foreach ($this->extractAiMateConfig($rootComposer, 'skills') as $dir) { - // TODO: replace the manual ".." check with PathGuard::hasTraversal() once #2245 is merged. - if ('' === trim($dir) || str_contains($dir, '..')) { + if ('' === trim($dir) || PathGuard::hasTraversal($dir)) { continue; } @@ -458,8 +457,7 @@ private function extractSkillsDirs(array $package, string $packageName): array $validDirs = []; foreach ($skillsDirs as $dir) { - // TODO: replace the manual ".." check with PathGuard::hasTraversal() once #2245 is merged. - if (!\is_string($dir) || '' === trim($dir) || str_contains($dir, '..')) { + if (!\is_string($dir) || '' === trim($dir) || PathGuard::hasTraversal($dir)) { continue; } diff --git a/src/mate/src/Service/SkillsInstaller.php b/src/mate/src/Service/SkillsInstaller.php index 79ac202f8a..b4c35ee1f1 100644 --- a/src/mate/src/Service/SkillsInstaller.php +++ b/src/mate/src/Service/SkillsInstaller.php @@ -13,6 +13,7 @@ use Psr\Log\LoggerInterface; use Symfony\AI\Mate\Discovery\ComposerExtensionDiscovery; +use Symfony\AI\Mate\Discovery\PathGuard; use Symfony\Component\Filesystem\Filesystem; /** @@ -118,10 +119,9 @@ private function findSkills(string $skillsPath): array $skills = []; foreach ($entries as $entry) { - // "." and ".." are scandir's own entries. Skill names are single path segments - // so they cannot traverse, but once #2245 (PathGuard) is merged route them through - // PathGuard::hasTraversal() to keep traversal handling consistent with discovery. - if ('.' === $entry || '..' === $entry) { + // "." is scandir's self-reference; PathGuard::hasTraversal() rejects ".." (and any + // other traversal) so handling stays consistent with discovery. + if ('.' === $entry || PathGuard::hasTraversal($entry)) { continue; } From c34c84d4fded6fc6f223c6bdc14fcd6b5ebbea2c Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Wed, 1 Jul 2026 22:18:54 +0200 Subject: [PATCH 3/4] [Mate] Harden skills:install against symlinks and stale links Follow-up hardening for the unreleased skills:install feature: - Security: reject a skill whose directory is a symlink or whose tree contains any symlink. Since a skill directory is symlinked wholesale into .agents/skills and .claude/skills, a nested symlink (e.g. a "secrets" entry pointing at ~/.ssh) would otherwise be followed by the coding agent, letting a malicious package surface out-of-tree files. - Prune Mate-managed mate-* skill links that are no longer discovered (extension or its skills removed) instead of leaving dangling links; only symlinks are removed, real user files/dirs are never touched. - Self-heal a dangling or incorrect source-of-truth link, symmetric with the existing mirror repair. Adds regression tests covering symlink rejection, stale-link pruning, user-owned mirror entries, relative link targets, dangling-link repair and the duplicate-name warning. --- src/mate/src/Service/SkillsInstaller.php | 93 +++++++++++++- .../tests/Service/SkillsInstallerTest.php | 117 ++++++++++++++++++ 2 files changed, 207 insertions(+), 3 deletions(-) diff --git a/src/mate/src/Service/SkillsInstaller.php b/src/mate/src/Service/SkillsInstaller.php index b4c35ee1f1..c2b654f40a 100644 --- a/src/mate/src/Service/SkillsInstaller.php +++ b/src/mate/src/Service/SkillsInstaller.php @@ -80,9 +80,19 @@ public function install(array $extensions): array } $target = $sourceRoot.'/'.$dirName; + $relativeTarget = rtrim($this->filesystem->makePathRelative($skillSource, $sourceRoot), '/'); - if (file_exists($target) || is_link($target)) { - // Already installed; still track it so the mirror is refreshed. + if (is_link($target)) { + if (file_exists($target) && readlink($target) === $relativeTarget) { + // Correct, resolvable link already in place. + $installedDirs[] = $dirName; + continue; + } + + // A wrong or dangling link we own — repair it below. + $this->filesystem->remove($target); + } elseif (file_exists($target)) { + // A real file or directory we do not own — leave it untouched. $installedDirs[] = $dirName; continue; } @@ -94,8 +104,12 @@ public function install(array $extensions): array } } + // Drop Mate-managed links whose skill is no longer discovered (extension/skills removed). + $this->pruneStale($installedDirs, $sourceRoot); + foreach ($this->skillMirrors as $mirrorDir) { $this->refreshMirror($installedDirs, $mirrorDir); + $this->pruneStale($installedDirs, $this->rootDir.'/'.trim($mirrorDir, '/')); } return $result; @@ -126,7 +140,16 @@ private function findSkills(string $skillsPath): array } $skillDir = $skillsPath.'/'.$entry; - if (!is_dir($skillDir) || !is_file($skillDir.'/SKILL.md')) { + if (is_link($skillDir) || !is_dir($skillDir) || !is_file($skillDir.'/SKILL.md')) { + continue; + } + + // A distributed skill is plain files (markdown, scripts, references). Any symlink in the + // tree would be exposed verbatim through the installed link, letting a malicious package + // surface files outside its own directory (e.g. a "secrets" entry pointing at ~/.ssh) to + // the coding agent. Reject such skills wholesale. + if ($this->containsSymlink($skillDir)) { + $this->logger->warning('Skipping skill containing symlinks', ['skill' => $entry]); continue; } @@ -136,6 +159,35 @@ private function findSkills(string $skillsPath): array return $skills; } + /** + * Recursively report whether a directory tree contains any symlink, without following links. + */ + private function containsSymlink(string $dir): bool + { + $entries = scandir($dir); + if (false === $entries) { + // Unreadable directory; treat as unsafe rather than silently installing it. + return true; + } + + foreach ($entries as $entry) { + if ('.' === $entry || PathGuard::hasTraversal($entry)) { + continue; + } + + $path = $dir.'/'.$entry; + if (is_link($path)) { + return true; + } + + if (is_dir($path) && $this->containsSymlink($path)) { + return true; + } + } + + return false; + } + private function placeSkill(string $source, string $target): void { $parent = \dirname($target); @@ -184,4 +236,39 @@ private function refreshMirror(array $skillDirs, string $mirrorDir): void $this->filesystem->symlink($relativeTarget, $mirrorSkill); } } + + /** + * Remove Mate-managed skill symlinks under $root that are no longer installed, so coding agents + * do not keep seeing dangling skills after an extension (or its skills directory) is removed. + * + * Only symlinks are removed; real files or directories a user placed are never touched. + * + * @param string[] $installedDirs prefixed skill directory names currently installed + */ + private function pruneStale(array $installedDirs, string $root): void + { + if (!is_dir($root)) { + return; + } + + $entries = scandir($root); + if (false === $entries) { + return; + } + + foreach ($entries as $entry) { + if ('.' === $entry || PathGuard::hasTraversal($entry)) { + continue; + } + + if (!str_starts_with($entry, 'mate-') || \in_array($entry, $installedDirs, true)) { + continue; + } + + $path = $root.'/'.$entry; + if (is_link($path)) { + $this->filesystem->remove($path); + } + } + } } diff --git a/src/mate/tests/Service/SkillsInstallerTest.php b/src/mate/tests/Service/SkillsInstallerTest.php index c39187b784..fb3e11feb0 100644 --- a/src/mate/tests/Service/SkillsInstallerTest.php +++ b/src/mate/tests/Service/SkillsInstallerTest.php @@ -12,6 +12,7 @@ namespace Symfony\AI\Mate\Tests\Service; use PHPUnit\Framework\TestCase; +use Psr\Log\AbstractLogger; use Psr\Log\NullLogger; use Symfony\AI\Mate\Service\SkillsInstaller; use Symfony\Component\Filesystem\Filesystem; @@ -151,6 +152,122 @@ public function testRootProjectSkillsAreInstalled() $this->assertTrue(is_link($this->rootDir.'/.claude/skills/mate-root-skill')); } + public function testRejectsSkillContainingSymlink() + { + // A malicious package ships a real skill dir whose tree hides a symlink pointing outside + // its own directory; installing it would surface the target to the coding agent. + $skillDir = $this->rootDir.'/vendor/vendor/evilpkg/skills/evil'; + $this->filesystem->mkdir($skillDir); + file_put_contents($skillDir.'/SKILL.md', "---\nname: evil\ndescription: x\n---\n"); + $secret = $this->rootDir.'/secret.txt'; + file_put_contents($secret, "TOP SECRET\n"); + symlink($secret, $skillDir.'/leak.txt'); + + $installer = $this->createInstaller(); + $result = $installer->install([ + 'vendor/evilpkg' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/evilpkg/skills']], + ]); + + $this->assertSame([], $result['installed']); + $this->assertDirectoryDoesNotExist($this->rootDir.'/.agents/skills/mate-evil'); + } + + public function testStaleSkillLinksArePruned() + { + $installer = $this->createInstaller(); + $installer->install($this->extensions()); + + $source = $this->rootDir.'/.agents/skills/mate-demo-skill'; + $mirror = $this->rootDir.'/.claude/skills/mate-demo-skill'; + $this->assertTrue(is_link($source)); + $this->assertTrue(is_link($mirror)); + + // The extension (and thus the skill) is no longer discovered on a subsequent run. + $installer->install([]); + + $this->assertFalse(is_link($source), 'Stale source link is pruned'); + $this->assertFalse(is_link($mirror), 'Stale mirror link is pruned'); + } + + public function testMirrorLeavesUserOwnedEntryUntouched() + { + $userDir = $this->rootDir.'/.claude/skills/mate-demo-skill'; + $this->filesystem->mkdir($userDir); + file_put_contents($userDir.'/user.txt', "mine\n"); + + $installer = $this->createInstaller(); + $installer->install($this->extensions()); + + $this->assertFalse(is_link($userDir), 'A real user-owned mirror entry is never replaced by a symlink'); + $this->assertStringEqualsFile($userDir.'/user.txt', "mine\n"); + } + + public function testInstalledSymlinksAreRelative() + { + $installer = $this->createInstaller(); + $installer->install($this->extensions()); + + $source = $this->rootDir.'/.agents/skills/mate-demo-skill'; + $mirror = $this->rootDir.'/.claude/skills/mate-demo-skill'; + + // Relative targets keep the committed .agents/.claude trees portable across machines. + $this->assertStringStartsWith('../', readlink($source)); + $this->assertStringContainsString('vendor/vendor/pkg/skills/demo-skill', readlink($source)); + $this->assertStringStartsWith('../', readlink($mirror)); + $this->assertStringContainsString('.agents/skills/mate-demo-skill', readlink($mirror)); + } + + public function testDanglingSourceLinkIsRepaired() + { + $installer = $this->createInstaller(); + $installer->install($this->extensions()); + + // Corrupt the source-of-truth link so it dangles. + $source = $this->rootDir.'/.agents/skills/mate-demo-skill'; + $this->filesystem->remove($source); + symlink('./does-not-exist', $source); + $this->assertFalse(file_exists($source), 'Source link is dangling before repair'); + + $installer->install($this->extensions()); + + $this->assertTrue(is_link($source)); + $this->assertFileExists($source.'/SKILL.md', 'Source link resolves to the installed skill again'); + } + + public function testDuplicateSkillNameLogsWarning() + { + $this->writeSkill('vendor/vendor/pkg-a/skills', 'shared', "from-a\n"); + $this->writeSkill('vendor/vendor/pkg-b/skills', 'shared', "from-b\n"); + + $logger = new class extends AbstractLogger { + /** + * @var list}> + */ + public array $records = []; + + public function log($level, string|\Stringable $message, array $context = []): void + { + $this->records[] = [$level, (string) $message, $context]; + } + }; + + $installer = new SkillsInstaller($this->rootDir, $logger, $this->filesystem, '.agents/skills', ['claude' => '.claude/skills']); + $installer->install([ + 'vendor/pkg-a' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/pkg-a/skills']], + 'vendor/pkg-b' => ['dirs' => [], 'includes' => [], 'skills' => ['vendor/vendor/pkg-b/skills']], + ]); + + $warnedForSharedSkill = false; + foreach ($logger->records as [, , $context]) { + if ('shared' === ($context['skill'] ?? null)) { + $warnedForSharedSkill = true; + break; + } + } + + $this->assertTrue($warnedForSharedSkill, 'A cross-extension name collision is logged with the skill name'); + } + /** * @param array $mirrors */ From 49b5a1da74992a273d57b309211b86f34e48157d Mon Sep 17 00:00:00 2001 From: Johannes Wachter Date: Wed, 1 Jul 2026 22:19:02 +0200 Subject: [PATCH 4/4] [Mate] Install root-project skills from discover and fix skills tests - discover now installs root-project (_custom) skills even when no Composer extensions are present, matching the standalone skills:install command instead of silently skipping them. - Add the required skills key to ExtensionData test fixtures (fixes PHPStan) and replace a tautological array-vs-count assertion. - Add discovery coverage for package skills traversal/absolute-path rejection and root-project skills extraction. --- src/mate/src/Command/DiscoverCommand.php | 7 +- .../Agent/AgentInstructionsAggregatorTest.php | 6 +- .../tests/Command/DiscoverCommandTest.php | 36 +++++++ .../ComposerExtensionDiscoveryTest.php | 98 ++++++++++++++++++- .../ExtensionConfigSynchronizerTest.php | 4 +- 5 files changed, 138 insertions(+), 13 deletions(-) diff --git a/src/mate/src/Command/DiscoverCommand.php b/src/mate/src/Command/DiscoverCommand.php index a200e2d534..dc7bfbc3cb 100644 --- a/src/mate/src/Command/DiscoverCommand.php +++ b/src/mate/src/Command/DiscoverCommand.php @@ -83,9 +83,9 @@ protected function execute(InputInterface $input, OutputInterface $output): int $count = \count($extensions); if (0 === $count) { - $materializationResult = $this->instructionsMaterializer->materializeForExtensions([ - '_custom' => $rootProjectExtension, - ]); + $rootOnlyExtensions = ['_custom' => $rootProjectExtension]; + $materializationResult = $this->instructionsMaterializer->materializeForExtensions($rootOnlyExtensions); + $skillResult = $this->syncSkills($rootOnlyExtensions); if ($composerMode) { $io->write('AI Mate: No extensions found.'); @@ -95,6 +95,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int 'Packages must have "extra.ai-mate" configuration in their composer.json.', ]); $this->displayInstructionsStatus($io, $materializationResult); + $this->displaySkillStatus($io, $skillResult); $io->note('Run "composer require vendor/package" to install MCP extensions.'); } diff --git a/src/mate/tests/Agent/AgentInstructionsAggregatorTest.php b/src/mate/tests/Agent/AgentInstructionsAggregatorTest.php index d364ae038f..4299a9542d 100644 --- a/src/mate/tests/Agent/AgentInstructionsAggregatorTest.php +++ b/src/mate/tests/Agent/AgentInstructionsAggregatorTest.php @@ -52,7 +52,7 @@ public function testAggregatesExtensionInstructions() { $aggregator = new AgentInstructionsAggregator( $this->rootDir, - ['acme/ext' => ['dirs' => [], 'includes' => [], 'instructions' => 'INSTRUCTIONS.md']], + ['acme/ext' => ['dirs' => [], 'includes' => [], 'instructions' => 'INSTRUCTIONS.md', 'skills' => []]], new NullLogger(), ); @@ -66,7 +66,7 @@ public function testRejectsTraversingExtensionInstructions() { $aggregator = new AgentInstructionsAggregator( $this->rootDir, - ['acme/ext' => ['dirs' => [], 'includes' => [], 'instructions' => '../../../secret.md']], + ['acme/ext' => ['dirs' => [], 'includes' => [], 'instructions' => '../../../secret.md', 'skills' => []]], new NullLogger(), ); @@ -77,7 +77,7 @@ public function testRejectsTraversingRootProjectInstructions() { $aggregator = new AgentInstructionsAggregator( $this->rootDir, - ['_custom' => ['dirs' => [], 'includes' => [], 'instructions' => '../secret.md']], + ['_custom' => ['dirs' => [], 'includes' => [], 'instructions' => '../secret.md', 'skills' => []]], new NullLogger(), ); diff --git a/src/mate/tests/Command/DiscoverCommandTest.php b/src/mate/tests/Command/DiscoverCommandTest.php index 2208ffac6f..b86fb3bbf0 100644 --- a/src/mate/tests/Command/DiscoverCommandTest.php +++ b/src/mate/tests/Command/DiscoverCommandTest.php @@ -311,6 +311,42 @@ public function testInstallsSkillsFromEnabledExtensions() } } + public function testInstallsRootProjectSkillsWhenNoExtensions() + { + $tempDir = sys_get_temp_dir().'/mate-discover-test-'.uniqid(); + mkdir($tempDir.'/skills/root-skill', 0755, true); + file_put_contents($tempDir.'/skills/root-skill/SKILL.md', "---\nname: root-skill\ndescription: x\n---\n"); + file_put_contents($tempDir.'/composer.json', json_encode([ + 'name' => 'test/project', + 'extra' => ['ai-mate' => ['skills' => ['skills']]], + ])); + + try { + $logger = new NullLogger(); + $discoverer = new ComposerExtensionDiscovery($tempDir, $logger); + $synchronizer = new ExtensionConfigSynchronizer($tempDir); + $aggregator = new AgentInstructionsAggregator($tempDir, [], $logger); + $materializer = new AgentInstructionsMaterializer($tempDir, $aggregator, $logger); + $command = new DiscoverCommand( + $discoverer, + $synchronizer, + $materializer, + new SkillsInstaller($tempDir, $logger, new Filesystem(), '.agents/skills', ['claude' => '.claude/skills']), + ); + $tester = new CommandTester($command); + + $tester->execute([]); + + $this->assertSame(Command::SUCCESS, $tester->getStatusCode()); + $this->assertStringContainsString('No MCP extensions found', $tester->getDisplay()); + // Root-project skills are installed even though zero Composer extensions exist. + $this->assertFileExists($tempDir.'/.agents/skills/mate-root-skill/SKILL.md'); + $this->assertTrue(is_link($tempDir.'/.claude/skills/mate-root-skill')); + } finally { + $this->removeDirectory($tempDir); + } + } + private function createConfiguration(string $rootDir, string $tempDir): string { // Copy fixture to temp directory for testing diff --git a/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php b/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php index 8b7be01e44..737c51ad31 100644 --- a/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php +++ b/src/mate/tests/Discovery/ComposerExtensionDiscoveryTest.php @@ -173,6 +173,47 @@ public function testIgnoresMissingSkillsDir() $this->assertSame([], $extensions['vendor/package-with-includes']['skills']); } + public function testDropsTraversingPackageSkillsDir() + { + $rootDir = sys_get_temp_dir().'/mate-skills-traversal-'.uniqid(); + mkdir($rootDir.'/vendor/composer', 0755, true); + mkdir($rootDir.'/vendor/acme/pkg/skills', 0755, true); + // Traversal target exists on disk, so only the segment-based guard — not the is_dir check — rejects it. + mkdir($rootDir.'/vendor/acme/escape', 0755, true); + file_put_contents($rootDir.'/vendor/composer/installed.json', '{"packages":[{"name":"acme/pkg","extra":{"ai-mate":{"skills":["../escape","skills"]}}}]}'); + + try { + $discovery = new ComposerExtensionDiscovery($rootDir, new NullLogger()); + + $extensions = $discovery->discover(); + + $this->assertArrayHasKey('acme/pkg', $extensions); + $this->assertSame(['vendor/acme/pkg/skills'], $extensions['acme/pkg']['skills']); + } finally { + (new Filesystem())->remove($rootDir); + } + } + + public function testStripsAbsolutePackageSkillsDir() + { + $rootDir = sys_get_temp_dir().'/mate-skills-absolute-'.uniqid(); + mkdir($rootDir.'/vendor/composer', 0755, true); + mkdir($rootDir.'/vendor/acme/pkg/abs', 0755, true); + file_put_contents($rootDir.'/vendor/composer/installed.json', '{"packages":[{"name":"acme/pkg","extra":{"ai-mate":{"skills":["/abs"]}}}]}'); + + try { + $discovery = new ComposerExtensionDiscovery($rootDir, new NullLogger()); + + $extensions = $discovery->discover(); + + // A leading slash is neutralized and the path stays contained under the package's vendor dir. + $this->assertArrayHasKey('acme/pkg', $extensions); + $this->assertSame(['vendor/acme/pkg/abs'], $extensions['acme/pkg']['skills']); + } finally { + (new Filesystem())->remove($rootDir); + } + } + public function testHandlesMissingInstalledJson() { $discovery = new ComposerExtensionDiscovery( @@ -195,7 +236,8 @@ public function testHandlesPackagesWithoutType() $extensions = $discovery->discover(); // Should discover packages with ai-mate config regardless of type field - $this->assertGreaterThanOrEqual(1, $extensions); + $this->assertGreaterThanOrEqual(1, \count($extensions)); + $this->assertArrayHasKey('vendor/package-mixed', $extensions); } public function testDiscoverRootProjectReturnsEmptyWhenComposerJsonDoesNotExist() @@ -207,11 +249,12 @@ public function testDiscoverRootProjectReturnsEmptyWhenComposerJsonDoesNotExist( $result = $discovery->discoverRootProject(); - $this->assertIsArray($result); $this->assertArrayHasKey('dirs', $result); $this->assertArrayHasKey('includes', $result); + $this->assertArrayHasKey('skills', $result); $this->assertSame([], $result['dirs']); $this->assertSame([], $result['includes']); + $this->assertSame([], $result['skills']); } public function testDiscoverRootProjectWithAiMateConfig() @@ -236,11 +279,12 @@ public function testDiscoverRootProjectWithAiMateConfig() $discovery = new ComposerExtensionDiscovery($tempDir, new NullLogger()); $result = $discovery->discoverRootProject(); - $this->assertIsArray($result); $this->assertArrayHasKey('dirs', $result); $this->assertArrayHasKey('includes', $result); + $this->assertArrayHasKey('skills', $result); $this->assertSame(['src', 'lib'], $result['dirs']); $this->assertSame(['config/mate.php'], $result['includes']); + $this->assertSame([], $result['skills']); } finally { unlink($tempDir.'/composer.json'); rmdir($tempDir); @@ -263,17 +307,60 @@ public function testDiscoverRootProjectWithoutAiMateConfig() $discovery = new ComposerExtensionDiscovery($tempDir, new NullLogger()); $result = $discovery->discoverRootProject(); - $this->assertIsArray($result); $this->assertArrayHasKey('dirs', $result); $this->assertArrayHasKey('includes', $result); + $this->assertArrayHasKey('skills', $result); $this->assertSame([], $result['dirs']); $this->assertSame([], $result['includes']); + $this->assertSame([], $result['skills']); } finally { unlink($tempDir.'/composer.json'); rmdir($tempDir); } } + public function testDiscoverRootProjectExtractsSkills() + { + $tempDir = sys_get_temp_dir().'/mate-root-skills-'.uniqid(); + mkdir($tempDir.'/skills', 0755, true); + + $composerJson = [ + 'name' => 'test/project', + 'extra' => ['ai-mate' => ['skills' => ['skills']]], + ]; + file_put_contents($tempDir.'/composer.json', json_encode($composerJson)); + + try { + $discovery = new ComposerExtensionDiscovery($tempDir, new NullLogger()); + $result = $discovery->discoverRootProject(); + + $this->assertSame(['skills'], $result['skills']); + } finally { + (new Filesystem())->remove($tempDir); + } + } + + public function testDiscoverRootProjectDropsMissingAndTraversingSkills() + { + $tempDir = sys_get_temp_dir().'/mate-root-skills-invalid-'.uniqid(); + mkdir($tempDir, 0755, true); + + $composerJson = [ + 'name' => 'test/project', + 'extra' => ['ai-mate' => ['skills' => ['does-not-exist', '../escape']]], + ]; + file_put_contents($tempDir.'/composer.json', json_encode($composerJson)); + + try { + $discovery = new ComposerExtensionDiscovery($tempDir, new NullLogger()); + $result = $discovery->discoverRootProject(); + + $this->assertSame([], $result['skills']); + } finally { + (new Filesystem())->remove($tempDir); + } + } + public function testDiscoverSkipsPackagesWithExtensionFalse() { $discovery = new ComposerExtensionDiscovery( @@ -389,11 +476,12 @@ public function testRootProjectNotAffectedByExtensionFlag() $result = $discovery->discoverRootProject(); // Root project discovery should work regardless of extension flag - $this->assertIsArray($result); $this->assertArrayHasKey('dirs', $result); $this->assertArrayHasKey('includes', $result); + $this->assertArrayHasKey('skills', $result); $this->assertSame(['src', 'lib'], $result['dirs']); $this->assertSame(['config/mate.php'], $result['includes']); + $this->assertSame([], $result['skills']); } finally { unlink($tempDir.'/composer.json'); rmdir($tempDir); diff --git a/src/mate/tests/Service/ExtensionConfigSynchronizerTest.php b/src/mate/tests/Service/ExtensionConfigSynchronizerTest.php index 32e469fc58..705ef83900 100644 --- a/src/mate/tests/Service/ExtensionConfigSynchronizerTest.php +++ b/src/mate/tests/Service/ExtensionConfigSynchronizerTest.php @@ -44,7 +44,7 @@ public function testWritesValidPackageNames() { $synchronizer = new ExtensionConfigSynchronizer($this->rootDir); - $synchronizer->synchronize(['vendor/package-a' => ['dirs' => [], 'includes' => []]]); + $synchronizer->synchronize(['vendor/package-a' => ['dirs' => [], 'includes' => [], 'skills' => []]]); $extensions = include $this->rootDir.'/mate/extensions.php'; @@ -59,7 +59,7 @@ public function testMaliciousPackageNameCannotInjectCode() // and inject additional array entries / PHP code under naive interpolation. $maliciousName = "evil', 'injected' => ['enabled' => true], 'x"; - $synchronizer->synchronize([$maliciousName => ['dirs' => [], 'includes' => []]]); + $synchronizer->synchronize([$maliciousName => ['dirs' => [], 'includes' => [], 'skills' => []]]); $extensions = include $this->rootDir.'/mate/extensions.php';