From 781cdc47493ff5714fc7ee85d697308af715f3b4 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 21 Oct 2026 13:33:24 +0200 Subject: [PATCH 1/8] feat(vgv_ai_cli): add --plugin to install Narrow an install to the pinned plugins it names: resolve only those, translate from them, and enable only them for Claude Code. The manifest keeps the version it recorded for a plugin a run skipped, and doctor notes a pinned plugin that was never installed instead of calling it stale. --- packages/vgv_ai_cli/README.md | 13 +++- .../src/commands/doctor/doctor_command.dart | 3 +- .../commands/doctor/model/plugin_version.dart | 6 +- .../lib/src/commands/install_command.dart | 40 +++++++++++-- .../lib/src/resolve/plugin_resolver.dart | 20 ++++--- .../lib/src/translate/agents_vars.dart | 8 ++- .../lib/src/translate/harness_translator.dart | 6 +- .../lib/src/translate/translator.dart | 4 ++ .../translators/antigravity_translator.dart | 23 ++++---- .../translators/claude_translator.dart | 23 ++++---- .../translators/codex_translator.dart | 17 +++--- .../translators/copilot_translator.dart | 23 ++++---- .../translators/gemini_translator.dart | 17 +++--- .../translators/opencode_translator.dart | 37 ++++++------ .../vgv_ai_cli/test/pinned_plugins_test.dart | 1 + .../commands/doctor/doctor_command_test.dart | 11 ++-- .../doctor/model/plugin_version_test.dart | 2 +- .../src/commands/install_command_test.dart | 59 +++++++++++++++++++ .../src/resolve/plugin_resolver_test.dart | 17 ++++++ .../test/src/translate/agents_vars_test.dart | 1 + .../test/src/translate/translator_test.dart | 1 + .../antigravity_translator_test.dart | 1 + .../translators/claude_translator_test.dart | 16 ++++- .../translators/codex_translator_test.dart | 1 + .../translators/copilot_translator_test.dart | 1 + .../translators/gemini_translator_test.dart | 4 +- .../translators/opencode_translator_test.dart | 1 + .../translators/pi_translator_test.dart | 2 +- 28 files changed, 260 insertions(+), 98 deletions(-) diff --git a/packages/vgv_ai_cli/README.md b/packages/vgv_ai_cli/README.md index bf8f1611..f3b544fe 100644 --- a/packages/vgv_ai_cli/README.md +++ b/packages/vgv_ai_cli/README.md @@ -65,6 +65,9 @@ vgv_ai install --no-claude # Also overwrite your values that conflict with the ones install writes vgv_ai install --force + +# Install just one of the pinned VGV plugins +vgv_ai install --plugin vgv-wingspan ``` `install` reads the pinned plugins from the Claude Code plugin cache when it @@ -82,6 +85,9 @@ how `doctor` and `uninstall` tell what's yours from what's VGV's. are written whole. A file you edited since `install` wrote it, or one at the same path that `install` didn't write, is kept unless you pass `--force`. - **Selection:** `--only` and `--no-` can't be combined. +- **Plugins:** without `--plugin`, every pinned plugin is installed. Naming one + leaves what a previous `install` wrote for the others in place; `uninstall` + removes that. ### doctor @@ -100,9 +106,10 @@ vgv_ai doctor --no-claude For each harness, `doctor` reports the files and keys that are missing, were changed since, or differ from what this version installs. It also compares the -plugin versions you installed from with the ones this version pins, suggests -configuring a harness that's installed but not configured, and warns about a -configured harness that's no longer installed. +plugin versions you installed from with the ones this version pins, notes a +pinned plugin you haven't installed, suggests configuring a harness that's +installed but not configured, and warns about a configured harness that's no +longer installed. Every problem comes with the `install` command that fixes it, and the report ends with one command that fixes everything `install` can, such as diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index 0081940d..404267f7 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -167,10 +167,11 @@ class DoctorCommand({ ) async { final progress = _logger.progress('Resolving plugins'); switch (await _resolver.resolve(home)) { - case ResolvedAssets(:final assets): + case ResolvedAssets(:final assets, :final plugins): progress.complete('Resolved plugins'); final translation = await _translator.translate( assets: assets, + plugins: plugins, home: home, selected: selected, ); diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart index 66faf6b3..84968034 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart @@ -67,8 +67,10 @@ class const PluginVersion._({ PluginStatus.newer => Finding.stale( '$name $installed installed, newer than the pinned $pinned', ), - PluginStatus.unrecorded => Finding.stale( - '$name not recorded, $pinned pinned', + // A plugin the manifest never recorded was not installed, which + // `install --plugin` makes a choice rather than a fault. + PluginStatus.unrecorded => Finding.suggestion( + '$name $pinned not installed', ), }; } diff --git a/packages/vgv_ai_cli/lib/src/commands/install_command.dart b/packages/vgv_ai_cli/lib/src/commands/install_command.dart index 6d3c08af..19cc0f0b 100644 --- a/packages/vgv_ai_cli/lib/src/commands/install_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/install_command.dart @@ -4,6 +4,7 @@ import 'package:args/command_runner.dart'; import 'package:collection/collection.dart'; import 'package:file/file.dart'; import 'package:mason_logger/mason_logger.dart'; +import 'package:plugin_source/plugin_source.dart'; import 'package:vgv_ai_cli/src/commands/harness_selection.dart'; import 'package:vgv_ai_cli/src/commands/reporting_manifest_store.dart'; import 'package:vgv_ai_cli/src/commands/user_home.dart'; @@ -22,6 +23,8 @@ import 'package:vgv_ai_cli/src/translate/translate.dart'; /// * `--only` configures the named harnesses, detected or not. /// * `--no-` skips one detected harness. It cannot be combined with /// `--only`, which already names the set. +/// * `--plugin` installs just the named VGV plugins instead of every pinned +/// one. /// {@endtemplate} class InstallCommand({ required final Logger _logger, @@ -45,6 +48,11 @@ class InstallCommand({ 'Also overwrite your values that conflict with the ones install ' 'writes.', negatable: false, + ) + ..addMultiOption( + 'plugin', + help: 'Install just these VGV plugins, instead of every pinned one.', + allowed: [for (final plugin in vgvPlugins) plugin.name], ); addHarnessOptions('Configure the Very Good AI tooling for'); } @@ -117,6 +125,7 @@ class InstallCommand({ final plan = await _translator.translate( assets: resolved.assets, + plugins: resolved.plugins, home: home, selected: selected, ); @@ -134,9 +143,9 @@ class InstallCommand({ dryRun || store.save( from: manifest, - to: reconciliation.manifest.withPlugins({ - for (final plugin in resolved.plugins) plugin.name: plugin.version, - }), + to: reconciliation.manifest.withPlugins( + _pluginVersions(manifest, resolved.plugins), + ), ); final anyChangeFailed = applied.any( (change) => change.outcome == ChangeOutcome.failed, @@ -185,6 +194,29 @@ class InstallCommand({ return selected; } + /// The plugins `--plugin` names, or null when it names none and every + /// pinned plugin is installed. + Set? get _selectedPlugins { + final names = argResults!['plugin'] as List; + return names.isEmpty ? null : names.toSet(); + } + + /// The versions to record for the pinned plugins: [installed] for the ones + /// this run wrote, and what [manifest] already records for the ones it + /// skipped, whose files are still installed. + Map _pluginVersions( + InstallManifest manifest, + List installed, + ) { + final written = { + for (final plugin in installed) plugin.name: plugin.version, + }; + return { + for (final plugin in vgvPlugins) + plugin.name: ?(written[plugin.name] ?? manifest.plugins[plugin.name]), + }; + } + /// Resolves and parses every pinned VGV plugin into one asset bundle. /// /// Returns `null`, having said why, when any plugin cannot be resolved or @@ -192,7 +224,7 @@ class InstallCommand({ Future _resolveAssets(Directory home) async { final progress = _logger.progress('Resolving plugins'); - switch (await _resolver.resolve(home)) { + switch (await _resolver.resolve(home, only: _selectedPlugins)) { case final ResolvedAssets resolved: progress.complete( 'Resolved ${resolved.assets.skills.length} skills and ' diff --git a/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart b/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart index c11cb18e..860660cd 100644 --- a/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart +++ b/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart @@ -18,10 +18,10 @@ class const PluginResolver({ final PluginParser _parser = const ClaudePluginParser(), final List _plugins = vgvPlugins, }) { - /// Resolves every plugin for [home], stopping at the first that cannot be - /// resolved or parsed: a half-configured harness is worse than an - /// unconfigured one. - Future resolve(Directory home) async { + /// Resolves the plugins [only] names for [home], or every pinned one when + /// it names none, stopping at the first that cannot be resolved or parsed: + /// a half-configured harness is worse than an unconfigured one. + Future resolve(Directory home, {Set? only}) async { final sources = [ LocalCachePluginSource(home: home), GitPluginSource( @@ -30,8 +30,13 @@ class const PluginResolver({ ), ]; + final plugins = [ + for (final plugin in _plugins) + if (only?.contains(plugin.name) ?? true) plugin, + ]; + final parsed = []; - for (final plugin in _plugins) { + for (final plugin in plugins) { switch (await _resolveOne(plugin, sources)) { case ResolvedAssets(:final assets): parsed.add(assets); @@ -42,10 +47,7 @@ class const PluginResolver({ } } - return ResolvedAssets( - assets: PluginAssets.merge(parsed), - plugins: _plugins, - ); + return ResolvedAssets(assets: PluginAssets.merge(parsed), plugins: plugins); } /// [plugin] parsed from the first of [sources] holding a copy that parses. diff --git a/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart b/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart index d34422d6..8ea08705 100644 --- a/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart +++ b/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart @@ -15,6 +15,9 @@ class const AgentsVars({ /// What the plugins contribute. required final PluginAssets assets, + /// The plugins being installed. + required final List plugins, + /// Where the shared skills are written. required final String skillsRoot, @@ -35,7 +38,10 @@ class const AgentsVars({ for (final harness in Harness.values) harness.name: translators.any((t) => t.harness == harness), for (final translator in translators) ...{ - '${translator.harness.name}_settings': ?translator.settings(assets), + '${translator.harness.name}_settings': ?translator.settings( + assets, + plugins: plugins, + ), '${translator.harness.name}_hooks': ?hooks[translator.harness], }, if (agents.isNotEmpty) 'agents': agents, diff --git a/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart b/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart index 1f697863..b7d4721b 100644 --- a/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart @@ -34,7 +34,11 @@ abstract class const HarnessTranslator() { /// The settings fragment merged into the root of this harness's config, /// or null for a harness with nothing to set, whose brick folder then has /// no settings file. - String? settings(PluginAssets assets) => null; + /// + /// [plugins] are the ones being installed, which only a harness that + /// installs them itself names in its settings. + String? settings(PluginAssets assets, {required List plugins}) => + null; /// What the brick renders [agent] with for this harness, under its name. /// diff --git a/packages/vgv_ai_cli/lib/src/translate/translator.dart b/packages/vgv_ai_cli/lib/src/translate/translator.dart index 30fc69d9..890e056b 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translator.dart @@ -20,8 +20,11 @@ class const Translator({ }) { /// The changes and notes for each of [selected], plus the shared skills /// when any of them reads them. + /// + /// [plugins] are the ones [assets] were resolved from. Future translate({ required PluginAssets assets, + required List plugins, required Directory home, required Set selected, }) async { @@ -39,6 +42,7 @@ class const Translator({ final vars = AgentsVars( translators: chosen, assets: assets, + plugins: plugins, skillsRoot: agentsSkillsRoot(home).path, hooks: { for (final MapEntry(key: harness, value: translation) in hooks.entries) diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart index c1218848..128a80c1 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart @@ -49,18 +49,19 @@ class const AntigravityTranslator() extends HarnessTranslator { /// The plugin's `mcp_config.json`. A remote server's endpoint is /// `serverUrl`. @override - String settings(PluginAssets assets) => jsonEncode({ - 'mcpServers': { - for (final server in assets.mcpServers) - server.name: switch (server) { - LocalMCPServer() => server.toJson(), - RemoteMCPServer(:final url, :final headers) => { - 'serverUrl': url, - if (headers.isNotEmpty) 'headers': headers, - }, + String settings(PluginAssets assets, {required List plugins}) => + jsonEncode({ + 'mcpServers': { + for (final server in assets.mcpServers) + server.name: switch (server) { + LocalMCPServer() => server.toJson(), + RemoteMCPServer(:final url, :final headers) => { + 'serverUrl': url, + if (headers.isNotEmpty) 'headers': headers, + }, + }, }, - }, - }); + }); /// An agent names its skills in the frontmatter, relative to the plugin, /// so its instructions are the body alone rather than the shared pointer. diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart index c3aeb6e6..e4c1dde7 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart @@ -22,17 +22,18 @@ class const ClaudeTranslator() extends HarnessTranslator { bool get readsSharedSkills => false; @override - String settings(PluginAssets assets) => jsonEncode({ - 'extraKnownMarketplaces': { - vgvMarketplaceName: { - 'source': {'source': 'github', 'repo': vgvMarketplaceRepo}, - }, - }, - 'enabledPlugins': { - for (final plugin in vgvPlugins) - '${plugin.name}@$vgvMarketplaceName': true, - }, - }); + String settings(PluginAssets assets, {required List plugins}) => + jsonEncode({ + 'extraKnownMarketplaces': { + vgvMarketplaceName: { + 'source': {'source': 'github', 'repo': vgvMarketplaceRepo}, + }, + }, + 'enabledPlugins': { + for (final plugin in plugins) + '${plugin.name}@$vgvMarketplaceName': true, + }, + }); /// Nothing to write: the plugin system runs the hooks as their authors /// wrote them. diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart index 32f6528f..51aacf0b 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart @@ -21,15 +21,16 @@ class const CodexTranslator() extends HarnessTranslator { /// Codex declares servers as `[mcp_servers.]` tables, and a remote /// server's static headers as `http_headers`. @override - String settings(PluginAssets assets) => TomlDocument.fromMap({ - 'mcp_servers': { - for (final server in assets.mcpServers) - server.name: { - for (final MapEntry(:key, :value) in server.toJson().entries) - _renamedKeys[key] ?? key: value, + String settings(PluginAssets assets, {required List plugins}) => + TomlDocument.fromMap({ + 'mcp_servers': { + for (final server in assets.mcpServers) + server.name: { + for (final MapEntry(:key, :value) in server.toJson().entries) + _renamedKeys[key] ?? key: value, + }, }, - }, - }).toString(); + }).toString(); /// Codex requires `name` and `developer_instructions`, and drops with a /// warning any agent declaring no `description`, so all three are written. diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart index 04315508..4b10c62f 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart @@ -31,18 +31,19 @@ class const CopilotTranslator() extends HarnessTranslator { /// Copilot documents a `type` on every server. @override - String settings(PluginAssets assets) => jsonEncode({ - 'mcpServers': { - for (final server in assets.mcpServers) - server.name: { - 'type': switch (server) { - LocalMCPServer() => 'local', - RemoteMCPServer() => 'http', - }, - ...server.toJson(), + String settings(PluginAssets assets, {required List plugins}) => + jsonEncode({ + 'mcpServers': { + for (final server in assets.mcpServers) + server.name: { + 'type': switch (server) { + LocalMCPServer() => 'local', + RemoteMCPServer() => 'http', + }, + ...server.toJson(), + }, }, - }, - }); + }); @override HarnessFields agent(AgentAsset agent, {required String instructions}) => diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart index 7f3ab911..2ee9ed1b 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart @@ -26,14 +26,15 @@ class const GeminiTranslator() extends HarnessTranslator { /// `AGENTS.md` is listed so Gemini reads the VGV section, and `GEMINI.md` /// so naming a second file does not stop it reading its default one. @override - String settings(PluginAssets assets) => jsonEncode({ - 'mcpServers': { - for (final server in assets.mcpServers) server.name: server.toJson(), - }, - 'context': { - 'fileName': ['AGENTS.md', 'GEMINI.md'], - }, - }); + String settings(PluginAssets assets, {required List plugins}) => + jsonEncode({ + 'mcpServers': { + for (final server in assets.mcpServers) server.name: server.toJson(), + }, + 'context': { + 'fileName': ['AGENTS.md', 'GEMINI.md'], + }, + }); @override HarnessFields agent(AgentAsset agent, {required String instructions}) => diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart index 0981110f..78387bcc 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart @@ -16,25 +16,26 @@ class const OpenCodeTranslator() extends HarnessTranslator { /// OpenCode types every server and folds a command's args into one array. @override - String settings(PluginAssets assets) => jsonEncode({ - r'$schema': 'https://opencode.ai/config.json', - 'mcp': { - for (final server in assets.mcpServers) - server.name: switch (server) { - LocalMCPServer(:final command, :final args, :final env) => { - 'type': 'local', - 'command': [command, ...args], - if (env.isNotEmpty) 'environment': env, - }, - RemoteMCPServer(:final url, :final headers) => { - 'type': 'remote', - 'url': url, - if (headers.isNotEmpty) 'headers': headers, - }, + String settings(PluginAssets assets, {required List plugins}) => + jsonEncode({ + r'$schema': 'https://opencode.ai/config.json', + 'mcp': { + for (final server in assets.mcpServers) + server.name: switch (server) { + LocalMCPServer(:final command, :final args, :final env) => { + 'type': 'local', + 'command': [command, ...args], + if (env.isNotEmpty) 'environment': env, + }, + RemoteMCPServer(:final url, :final headers) => { + 'type': 'remote', + 'url': url, + if (headers.isNotEmpty) 'headers': headers, + }, + }, }, - }, - 'instructions': ['AGENTS.md'], - }); + 'instructions': ['AGENTS.md'], + }); @override HarnessFields agent(AgentAsset agent, {required String instructions}) => diff --git a/packages/vgv_ai_cli/test/pinned_plugins_test.dart b/packages/vgv_ai_cli/test/pinned_plugins_test.dart index 3ed7f9c9..b4826b2f 100644 --- a/packages/vgv_ai_cli/test/pinned_plugins_test.dart +++ b/packages/vgv_ai_cli/test/pinned_plugins_test.dart @@ -43,6 +43,7 @@ void main() { test('translates $vgvPlugins for ${harness.name}', () async { final plan = await const Translator().translate( assets: assets, + plugins: vgvPlugins, home: home, selected: {harness}, ); diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index de06c498..92351ce3 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -35,7 +35,7 @@ class _FakeResolver extends Fake implements PluginResolver { int calls = 0; @override - Future resolve(Directory home) async { + Future resolve(Directory home, {Set? only}) async { calls++; return resolution; } @@ -50,6 +50,7 @@ class _FakeTranslator extends Fake implements Translator { @override Future translate({ required PluginAssets assets, + required List plugins, required Directory home, required Set selected, }) async { @@ -668,15 +669,13 @@ void main() { verifyInfo(' Run vgv_ai install --only codex'); }); - test('a pinned plugin that was never recorded exits 70', () async { + test('a pinned plugin that was never installed is only noted', () async { installWith({}); final exitCode = await runDoctor(); - expect(exitCode, ExitCode.software.code); - verifyInfo( - ' ${wingspan.name} not recorded, ${wingspan.version} pinned', - ); + expect(exitCode, ExitCode.success.code); + verifyInfo(' ${wingspan.name} ${wingspan.version} not installed'); }); test( diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart index 8c9519e9..1eaaba40 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart @@ -63,7 +63,7 @@ void main() { expect(version.status, PluginStatus.unrecorded); expect( version.finding, - isFinding(FindingKind.stale, 'wingspan not recorded, 0.2.0 pinned'), + isFinding(FindingKind.suggestion, 'wingspan 0.2.0 not installed'), ); }); }); diff --git a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart index b6116c47..a51cfc87 100644 --- a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart @@ -43,15 +43,18 @@ class _FakeTranslator extends Fake implements Translator { Set? selected; PluginAssets? assets; + List? plugins; Directory? home; @override Future translate({ required PluginAssets assets, + required List plugins, required Directory home, required Set selected, }) async { this.assets = assets; + this.plugins = plugins; this.home = home; this.selected = selected; return plan; @@ -246,6 +249,62 @@ void main() { }); }); + group('--plugin', () { + // The manifest is only written once something is, so every run here + // writes one file. + setUp(() { + translator.plan = TranslationPlan( + harnesses: { + Harness.claude: TranslationResult( + changes: [ownChange(Harness.claude, '.claude/a.md', 'a')], + ), + }, + ); + }); + + test('installs just the plugins it names', () async { + final exitCode = await runInstall([ + '--only', + 'claude', + '--plugin', + vgvPlugins.first.name, + ]); + + expect(exitCode, ExitCode.success.code); + expect(translator.plugins, [vgvPlugins.first]); + expect(ManifestStore(home).load()!.plugins, { + vgvPlugins.first.name: vgvPlugins.first.version, + }); + }); + + test('keeps the recorded version of a plugin it skips', () async { + await runInstall(['--only', 'claude']); + + await runInstall([ + '--only', + 'claude', + '--plugin', + vgvPlugins.first.name, + ]); + + expect(ManifestStore(home).load()!.plugins, { + for (final plugin in vgvPlugins) plugin.name: plugin.version, + }); + }); + + test('rejects a plugin it does not pin', () async { + final exitCode = await runInstall([ + '--only', + 'claude', + '--plugin', + 'vgv-unknown', + ]); + + expect(exitCode, ExitCode.usage.code); + expect(translator.plugins, isNull); + }); + }); + group('resolution', () { test('clones the plugins when there is no Claude Code cache', () async { final cloned = >[]; diff --git a/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart b/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart index e444d5c3..f12d6597 100644 --- a/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart +++ b/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart @@ -271,6 +271,23 @@ void main() { ); }); + test('resolves only the plugins `only` names', () async { + cache(wingspan); + final resolver = PluginResolver( + runProcess: processes.never, + parser: parser, + plugins: const [wingspan, flutter], + ); + + final resolution = await resolver.resolve(home, only: {wingspan.name}); + + expect(parser.roots, [cached(wingspan)]); + expect( + resolution, + isA().having((r) => r.plugins, 'plugins', [wingspan]), + ); + }); + test('resolves the pinned VGV plugins by default', () async { vgvPlugins.forEach(cache); final resolver = PluginResolver( diff --git a/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart b/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart index 487e388e..714c3c45 100644 --- a/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart @@ -17,6 +17,7 @@ void main() { }) => AgentsVars( translators: translators, assets: assets ?? pluginAssets(), + plugins: vgvPlugins, skillsRoot: '/home/dev/.agents/skills', hooks: hooks, ).toMap(); diff --git a/packages/vgv_ai_cli/test/src/translate/translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translator_test.dart index 5e1e19c6..14a912eb 100644 --- a/packages/vgv_ai_cli/test/src/translate/translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translator_test.dart @@ -47,6 +47,7 @@ void main() { PluginAssets? using, }) => const Translator().translate( assets: using ?? assets, + plugins: vgvPlugins, home: home, selected: selected, ); diff --git a/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart index ffcbd5bd..2b1a571c 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart @@ -56,6 +56,7 @@ void main() { ), ], ), + plugins: vgvPlugins, ); expect(jsonDecode(json), { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart index 3d9303bf..f78174a3 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart @@ -1,6 +1,7 @@ import 'dart:convert'; import 'package:file/memory.dart'; +import 'package:plugin_source/plugin_source.dart'; import 'package:test/test.dart'; import 'package:vgv_ai_cli/src/model/model.dart'; import 'package:vgv_ai_cli/src/translate/translate.dart'; @@ -16,7 +17,9 @@ void main() { }); test('points Claude Code at the marketplace and enables both plugins', () { - final settings = jsonDecode(translator.settings(pluginAssets())); + final settings = jsonDecode( + translator.settings(pluginAssets(), plugins: vgvPlugins), + ); expect(settings, { 'extraKnownMarketplaces': { @@ -34,6 +37,17 @@ void main() { }); }); + test('enables only the plugins it is given', () { + final wingspan = vgvPlugins.first; + final settings = jsonDecode( + translator.settings(pluginAssets(), plugins: [wingspan]), + ) as Map; + + expect(settings['enabledPlugins'], { + '${wingspan.name}@very-good-claude-code-marketplace': true, + }); + }); + test('spells no agents or commands, which the marketplace supplies', () { final fs = MemoryFileSystem.test(); diff --git a/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart index 23e178f2..eaaf8fdd 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart @@ -33,6 +33,7 @@ void main() { ), ], ), + plugins: vgvPlugins, ); expect(TomlDocument.parse(toml).toMap(), { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart index 0f7cfd96..45caedfa 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart @@ -34,6 +34,7 @@ void main() { ), ], ), + plugins: vgvPlugins, ); expect(jsonDecode(json), { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart index 3ecff067..8a66e75d 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart @@ -44,6 +44,7 @@ void main() { ), ], ), + plugins: vgvPlugins, ); expect( @@ -68,6 +69,7 @@ void main() { ), ], ), + plugins: vgvPlugins, ); expect( @@ -83,7 +85,7 @@ void main() { test('reads AGENTS.md without giving up its own GEMINI.md', () { expect( - jsonDecode(translator.settings(pluginAssets())), + jsonDecode(translator.settings(pluginAssets(), plugins: vgvPlugins)), containsPair('context', { 'fileName': ['AGENTS.md', 'GEMINI.md'], }), diff --git a/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart index 3709a9d2..1c8db718 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart @@ -48,6 +48,7 @@ void main() { const RemoteMCPServer(name: 'open', url: 'https://open.example'), ], ), + plugins: vgvPlugins, ), ); diff --git a/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart index 82a381e3..3ef0a1bd 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart @@ -15,7 +15,7 @@ void main() { }); test('has no settings, since Pi reads the shared skills natively', () { - expect(translator.settings(pluginAssets()), isNull); + expect(translator.settings(pluginAssets(), plugins: vgvPlugins), isNull); }); test('spells nothing else, since Pi reads prompts as written', () { From c91103e3b5aa668c5be2818769e7f76f0565c7f5 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 21 Oct 2026 13:42:30 +0200 Subject: [PATCH 2/8] refactor: name the plugins in PluginAssets Credit each plugin for what it contributes, so the Claude translator reads the list off the assets it already gets. The other translators keep their signatures instead of taking a plugin list none of them uses. --- packages/plugin_source/README.md | 3 +- .../lib/src/model/plugin_assets.dart | 15 ++++++++ .../test/src/model/plugin_assets_test.dart | 34 +++++++++++++++++ .../src/commands/doctor/doctor_command.dart | 3 +- .../commands/doctor/model/plugin_version.dart | 2 - .../lib/src/commands/install_command.dart | 3 +- .../src/resolve/model/asset_resolution.dart | 5 +-- .../lib/src/resolve/plugin_resolver.dart | 5 +-- .../lib/src/translate/agents_vars.dart | 8 +--- .../lib/src/translate/harness_translator.dart | 6 +-- .../lib/src/translate/translator.dart | 4 -- .../translators/antigravity_translator.dart | 23 ++++++------ .../translators/claude_translator.dart | 23 ++++++------ .../translators/codex_translator.dart | 17 ++++----- .../translators/copilot_translator.dart | 23 ++++++------ .../translators/gemini_translator.dart | 17 ++++----- .../translators/opencode_translator.dart | 37 +++++++++---------- .../vgv_ai_cli/test/pinned_plugins_test.dart | 1 - .../commands/doctor/doctor_command_test.dart | 9 +++-- .../src/commands/install_command_test.dart | 7 +--- .../src/resolve/plugin_resolver_test.dart | 12 ++++-- .../test/src/translate/_assets.dart | 2 + .../test/src/translate/agents_vars_test.dart | 1 - .../test/src/translate/translator_test.dart | 1 - .../antigravity_translator_test.dart | 1 - .../translators/claude_translator_test.dart | 6 +-- .../translators/codex_translator_test.dart | 1 - .../translators/copilot_translator_test.dart | 1 - .../translators/gemini_translator_test.dart | 4 +- .../translators/opencode_translator_test.dart | 1 - .../translators/pi_translator_test.dart | 2 +- 31 files changed, 149 insertions(+), 128 deletions(-) diff --git a/packages/plugin_source/README.md b/packages/plugin_source/README.md index a22d75eb..d2c5d805 100644 --- a/packages/plugin_source/README.md +++ b/packages/plugin_source/README.md @@ -43,7 +43,8 @@ Future main() async { A source that can't supply the plugin returns an `UnavailablePlugin` with the reasons, rather than throwing. A plugin file that can't be understood makes the parser throw a `PluginParseException` that names the file and the field. -`PluginAssets.merge` combines several plugins into one set of assets. +`PluginAssets.merge` combines several plugins into one set of assets, and +`withPlugin` records which plugin supplied one. ## Layout diff --git a/packages/plugin_source/lib/src/model/plugin_assets.dart b/packages/plugin_source/lib/src/model/plugin_assets.dart index f672a35f..e948b857 100644 --- a/packages/plugin_source/lib/src/model/plugin_assets.dart +++ b/packages/plugin_source/lib/src/model/plugin_assets.dart @@ -2,6 +2,7 @@ import 'package:plugin_source/src/model/agent_asset.dart'; import 'package:plugin_source/src/model/hook_asset.dart'; import 'package:plugin_source/src/model/mcp_server.dart'; import 'package:plugin_source/src/model/skill_asset.dart'; +import 'package:plugin_source/src/plugin_ref.dart'; /// {@template plugin_assets} /// Everything one or more plugins contribute, in a harness-neutral shape. @@ -18,6 +19,10 @@ class const PluginAssets({ /// The hooks, in the order the plugins declare them. final List hooks = const [], + + /// The plugins these come from, which the harness that installs them + /// itself names in its settings. + final List plugins = const [], }) { /// Combines what several plugins contribute into one [PluginAssets]. /// @@ -31,6 +36,16 @@ class const PluginAssets({ for (final a in assets) ...a.mcpServers, ], (s) => s.name), hooks: [for (final a in assets) ...a.hooks], + plugins: [for (final a in assets) ...a.plugins], + ); + + /// These assets, with [plugin] credited for them. + PluginAssets withPlugin(PluginRef plugin) => PluginAssets( + skills: skills, + agents: agents, + mcpServers: mcpServers, + hooks: hooks, + plugins: [...plugins, plugin], ); /// The skills a user can invoke as a command. diff --git a/packages/plugin_source/test/src/model/plugin_assets_test.dart b/packages/plugin_source/test/src/model/plugin_assets_test.dart index d2d9a0e0..a1b9a18c 100644 --- a/packages/plugin_source/test/src/model/plugin_assets_test.dart +++ b/packages/plugin_source/test/src/model/plugin_assets_test.dart @@ -29,8 +29,33 @@ void main() { hooks: hooks, ); + const wingspan = PluginRef( + name: 'wingspan', + repo: 'acme/wingspan', + version: '1.0.0', + gitRef: 'v1.0.0', + ); + + const flutter = PluginRef( + name: 'flutter', + repo: 'acme/flutter', + version: '2.0.0', + gitRef: 'v2.0.0', + ); + setUp(() => fs = MemoryFileSystem.test()); + group('withPlugin', () { + test('credits the plugin for what it contributes', () { + final assets = pluginAssets(skills: [skill('green-gate')]); + + final credited = assets.withPlugin(wingspan); + + expect(credited.plugins, [wingspan]); + expect(credited.skills, assets.skills); + }); + }); + group('invocableSkills', () { test('keeps only the skills a user can invoke as a command', () { final assets = pluginAssets( @@ -57,6 +82,15 @@ void main() { }); group('merge', () { + test('names every plugin that contributed', () { + final merged = PluginAssets.merge([ + pluginAssets().withPlugin(wingspan), + pluginAssets().withPlugin(flutter), + ]); + + expect(merged.plugins, [wingspan, flutter]); + }); + AgentAsset agent(String name) => AgentAsset(name: name, description: 'd', body: 'b', skills: const []); diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index 404267f7..0081940d 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -167,11 +167,10 @@ class DoctorCommand({ ) async { final progress = _logger.progress('Resolving plugins'); switch (await _resolver.resolve(home)) { - case ResolvedAssets(:final assets, :final plugins): + case ResolvedAssets(:final assets): progress.complete('Resolved plugins'); final translation = await _translator.translate( assets: assets, - plugins: plugins, home: home, selected: selected, ); diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart index 84968034..72302786 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart @@ -67,8 +67,6 @@ class const PluginVersion._({ PluginStatus.newer => Finding.stale( '$name $installed installed, newer than the pinned $pinned', ), - // A plugin the manifest never recorded was not installed, which - // `install --plugin` makes a choice rather than a fault. PluginStatus.unrecorded => Finding.suggestion( '$name $pinned not installed', ), diff --git a/packages/vgv_ai_cli/lib/src/commands/install_command.dart b/packages/vgv_ai_cli/lib/src/commands/install_command.dart index 19cc0f0b..ed5c310f 100644 --- a/packages/vgv_ai_cli/lib/src/commands/install_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/install_command.dart @@ -125,7 +125,6 @@ class InstallCommand({ final plan = await _translator.translate( assets: resolved.assets, - plugins: resolved.plugins, home: home, selected: selected, ); @@ -144,7 +143,7 @@ class InstallCommand({ store.save( from: manifest, to: reconciliation.manifest.withPlugins( - _pluginVersions(manifest, resolved.plugins), + _pluginVersions(manifest, resolved.assets.plugins), ), ); final anyChangeFailed = applied.any( diff --git a/packages/vgv_ai_cli/lib/src/resolve/model/asset_resolution.dart b/packages/vgv_ai_cli/lib/src/resolve/model/asset_resolution.dart index 4587ca1b..285a071d 100644 --- a/packages/vgv_ai_cli/lib/src/resolve/model/asset_resolution.dart +++ b/packages/vgv_ai_cli/lib/src/resolve/model/asset_resolution.dart @@ -6,14 +6,11 @@ import 'package:plugin_source/plugin_source.dart'; sealed class const AssetResolution(); /// {@template resolved_assets} -/// Every plugin in [plugins] resolved and parsed, merged into [assets]. +/// Every plugin resolved and parsed, merged into [assets], which names them. /// {@endtemplate} final class const ResolvedAssets({ /// What the plugins contribute, merged. required final PluginAssets assets, - - /// The plugins that were resolved, at the versions they were pinned to. - required final List plugins, }) extends AssetResolution; /// {@template unavailable_assets} diff --git a/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart b/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart index 860660cd..151a2e47 100644 --- a/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart +++ b/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart @@ -47,7 +47,7 @@ class const PluginResolver({ } } - return ResolvedAssets(assets: PluginAssets.merge(parsed), plugins: plugins); + return ResolvedAssets(assets: PluginAssets.merge(parsed)); } /// [plugin] parsed from the first of [sources] holding a copy that parses. @@ -68,8 +68,7 @@ class const PluginResolver({ case ResolvedPlugin(:final directory): try { return ResolvedAssets( - assets: _parser.parse(directory), - plugins: [plugin], + assets: _parser.parse(directory).withPlugin(plugin), ); } on PluginParseException catch (error) { unparseable = UnparseableAssets(plugin, error); diff --git a/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart b/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart index 8ea08705..d34422d6 100644 --- a/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart +++ b/packages/vgv_ai_cli/lib/src/translate/agents_vars.dart @@ -15,9 +15,6 @@ class const AgentsVars({ /// What the plugins contribute. required final PluginAssets assets, - /// The plugins being installed. - required final List plugins, - /// Where the shared skills are written. required final String skillsRoot, @@ -38,10 +35,7 @@ class const AgentsVars({ for (final harness in Harness.values) harness.name: translators.any((t) => t.harness == harness), for (final translator in translators) ...{ - '${translator.harness.name}_settings': ?translator.settings( - assets, - plugins: plugins, - ), + '${translator.harness.name}_settings': ?translator.settings(assets), '${translator.harness.name}_hooks': ?hooks[translator.harness], }, if (agents.isNotEmpty) 'agents': agents, diff --git a/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart b/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart index b7d4721b..1f697863 100644 --- a/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/harness_translator.dart @@ -34,11 +34,7 @@ abstract class const HarnessTranslator() { /// The settings fragment merged into the root of this harness's config, /// or null for a harness with nothing to set, whose brick folder then has /// no settings file. - /// - /// [plugins] are the ones being installed, which only a harness that - /// installs them itself names in its settings. - String? settings(PluginAssets assets, {required List plugins}) => - null; + String? settings(PluginAssets assets) => null; /// What the brick renders [agent] with for this harness, under its name. /// diff --git a/packages/vgv_ai_cli/lib/src/translate/translator.dart b/packages/vgv_ai_cli/lib/src/translate/translator.dart index 890e056b..30fc69d9 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translator.dart @@ -20,11 +20,8 @@ class const Translator({ }) { /// The changes and notes for each of [selected], plus the shared skills /// when any of them reads them. - /// - /// [plugins] are the ones [assets] were resolved from. Future translate({ required PluginAssets assets, - required List plugins, required Directory home, required Set selected, }) async { @@ -42,7 +39,6 @@ class const Translator({ final vars = AgentsVars( translators: chosen, assets: assets, - plugins: plugins, skillsRoot: agentsSkillsRoot(home).path, hooks: { for (final MapEntry(key: harness, value: translation) in hooks.entries) diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart index 128a80c1..c1218848 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/antigravity_translator.dart @@ -49,19 +49,18 @@ class const AntigravityTranslator() extends HarnessTranslator { /// The plugin's `mcp_config.json`. A remote server's endpoint is /// `serverUrl`. @override - String settings(PluginAssets assets, {required List plugins}) => - jsonEncode({ - 'mcpServers': { - for (final server in assets.mcpServers) - server.name: switch (server) { - LocalMCPServer() => server.toJson(), - RemoteMCPServer(:final url, :final headers) => { - 'serverUrl': url, - if (headers.isNotEmpty) 'headers': headers, - }, - }, + String settings(PluginAssets assets) => jsonEncode({ + 'mcpServers': { + for (final server in assets.mcpServers) + server.name: switch (server) { + LocalMCPServer() => server.toJson(), + RemoteMCPServer(:final url, :final headers) => { + 'serverUrl': url, + if (headers.isNotEmpty) 'headers': headers, + }, }, - }); + }, + }); /// An agent names its skills in the frontmatter, relative to the plugin, /// so its instructions are the body alone rather than the shared pointer. diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart index e4c1dde7..1dae2f81 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/claude_translator.dart @@ -22,18 +22,17 @@ class const ClaudeTranslator() extends HarnessTranslator { bool get readsSharedSkills => false; @override - String settings(PluginAssets assets, {required List plugins}) => - jsonEncode({ - 'extraKnownMarketplaces': { - vgvMarketplaceName: { - 'source': {'source': 'github', 'repo': vgvMarketplaceRepo}, - }, - }, - 'enabledPlugins': { - for (final plugin in plugins) - '${plugin.name}@$vgvMarketplaceName': true, - }, - }); + String settings(PluginAssets assets) => jsonEncode({ + 'extraKnownMarketplaces': { + vgvMarketplaceName: { + 'source': {'source': 'github', 'repo': vgvMarketplaceRepo}, + }, + }, + 'enabledPlugins': { + for (final plugin in assets.plugins) + '${plugin.name}@$vgvMarketplaceName': true, + }, + }); /// Nothing to write: the plugin system runs the hooks as their authors /// wrote them. diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart index 51aacf0b..32f6528f 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/codex_translator.dart @@ -21,16 +21,15 @@ class const CodexTranslator() extends HarnessTranslator { /// Codex declares servers as `[mcp_servers.]` tables, and a remote /// server's static headers as `http_headers`. @override - String settings(PluginAssets assets, {required List plugins}) => - TomlDocument.fromMap({ - 'mcp_servers': { - for (final server in assets.mcpServers) - server.name: { - for (final MapEntry(:key, :value) in server.toJson().entries) - _renamedKeys[key] ?? key: value, - }, + String settings(PluginAssets assets) => TomlDocument.fromMap({ + 'mcp_servers': { + for (final server in assets.mcpServers) + server.name: { + for (final MapEntry(:key, :value) in server.toJson().entries) + _renamedKeys[key] ?? key: value, }, - }).toString(); + }, + }).toString(); /// Codex requires `name` and `developer_instructions`, and drops with a /// warning any agent declaring no `description`, so all three are written. diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart index 4b10c62f..04315508 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/copilot_translator.dart @@ -31,19 +31,18 @@ class const CopilotTranslator() extends HarnessTranslator { /// Copilot documents a `type` on every server. @override - String settings(PluginAssets assets, {required List plugins}) => - jsonEncode({ - 'mcpServers': { - for (final server in assets.mcpServers) - server.name: { - 'type': switch (server) { - LocalMCPServer() => 'local', - RemoteMCPServer() => 'http', - }, - ...server.toJson(), - }, + String settings(PluginAssets assets) => jsonEncode({ + 'mcpServers': { + for (final server in assets.mcpServers) + server.name: { + 'type': switch (server) { + LocalMCPServer() => 'local', + RemoteMCPServer() => 'http', + }, + ...server.toJson(), }, - }); + }, + }); @override HarnessFields agent(AgentAsset agent, {required String instructions}) => diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart index 2ee9ed1b..7f3ab911 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/gemini_translator.dart @@ -26,15 +26,14 @@ class const GeminiTranslator() extends HarnessTranslator { /// `AGENTS.md` is listed so Gemini reads the VGV section, and `GEMINI.md` /// so naming a second file does not stop it reading its default one. @override - String settings(PluginAssets assets, {required List plugins}) => - jsonEncode({ - 'mcpServers': { - for (final server in assets.mcpServers) server.name: server.toJson(), - }, - 'context': { - 'fileName': ['AGENTS.md', 'GEMINI.md'], - }, - }); + String settings(PluginAssets assets) => jsonEncode({ + 'mcpServers': { + for (final server in assets.mcpServers) server.name: server.toJson(), + }, + 'context': { + 'fileName': ['AGENTS.md', 'GEMINI.md'], + }, + }); @override HarnessFields agent(AgentAsset agent, {required String instructions}) => diff --git a/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart b/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart index 78387bcc..0981110f 100644 --- a/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart +++ b/packages/vgv_ai_cli/lib/src/translate/translators/opencode_translator.dart @@ -16,26 +16,25 @@ class const OpenCodeTranslator() extends HarnessTranslator { /// OpenCode types every server and folds a command's args into one array. @override - String settings(PluginAssets assets, {required List plugins}) => - jsonEncode({ - r'$schema': 'https://opencode.ai/config.json', - 'mcp': { - for (final server in assets.mcpServers) - server.name: switch (server) { - LocalMCPServer(:final command, :final args, :final env) => { - 'type': 'local', - 'command': [command, ...args], - if (env.isNotEmpty) 'environment': env, - }, - RemoteMCPServer(:final url, :final headers) => { - 'type': 'remote', - 'url': url, - if (headers.isNotEmpty) 'headers': headers, - }, - }, + String settings(PluginAssets assets) => jsonEncode({ + r'$schema': 'https://opencode.ai/config.json', + 'mcp': { + for (final server in assets.mcpServers) + server.name: switch (server) { + LocalMCPServer(:final command, :final args, :final env) => { + 'type': 'local', + 'command': [command, ...args], + if (env.isNotEmpty) 'environment': env, + }, + RemoteMCPServer(:final url, :final headers) => { + 'type': 'remote', + 'url': url, + if (headers.isNotEmpty) 'headers': headers, + }, }, - 'instructions': ['AGENTS.md'], - }); + }, + 'instructions': ['AGENTS.md'], + }); @override HarnessFields agent(AgentAsset agent, {required String instructions}) => diff --git a/packages/vgv_ai_cli/test/pinned_plugins_test.dart b/packages/vgv_ai_cli/test/pinned_plugins_test.dart index b4826b2f..3ed7f9c9 100644 --- a/packages/vgv_ai_cli/test/pinned_plugins_test.dart +++ b/packages/vgv_ai_cli/test/pinned_plugins_test.dart @@ -43,7 +43,6 @@ void main() { test('translates $vgvPlugins for ${harness.name}', () async { final plan = await const Translator().translate( assets: assets, - plugins: vgvPlugins, home: home, selected: {harness}, ); diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index 92351ce3..a015d3f9 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -28,8 +28,12 @@ class _MockProgress extends Mock implements Progress; /// A [PluginResolver] that answers with [resolution], and counts its calls. class _FakeResolver extends Fake implements PluginResolver { AssetResolution resolution = const ResolvedAssets( - assets: PluginAssets(skills: [], agents: [], mcpServers: []), - plugins: vgvPlugins, + assets: PluginAssets( + skills: [], + agents: [], + mcpServers: [], + plugins: vgvPlugins, + ), ); int calls = 0; @@ -50,7 +54,6 @@ class _FakeTranslator extends Fake implements Translator { @override Future translate({ required PluginAssets assets, - required List plugins, required Directory home, required Set selected, }) async { diff --git a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart index a51cfc87..a7873108 100644 --- a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart @@ -43,18 +43,15 @@ class _FakeTranslator extends Fake implements Translator { Set? selected; PluginAssets? assets; - List? plugins; Directory? home; @override Future translate({ required PluginAssets assets, - required List plugins, required Directory home, required Set selected, }) async { this.assets = assets; - this.plugins = plugins; this.home = home; this.selected = selected; return plan; @@ -271,7 +268,7 @@ void main() { ]); expect(exitCode, ExitCode.success.code); - expect(translator.plugins, [vgvPlugins.first]); + expect(translator.assets?.plugins, [vgvPlugins.first]); expect(ManifestStore(home).load()!.plugins, { vgvPlugins.first.name: vgvPlugins.first.version, }); @@ -301,7 +298,7 @@ void main() { ]); expect(exitCode, ExitCode.usage.code); - expect(translator.plugins, isNull); + expect(translator.assets, isNull); }); }); diff --git a/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart b/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart index f12d6597..31796fe6 100644 --- a/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart +++ b/packages/vgv_ai_cli/test/src/resolve/plugin_resolver_test.dart @@ -131,7 +131,7 @@ void main() { expect( resolution, isA() - .having((r) => r.plugins, 'plugins', [wingspan, flutter]) + .having((r) => r.assets.plugins, 'plugins', [wingspan, flutter]) .having( (r) => r.assets.agents.map((agent) => agent.name), 'agents', @@ -284,7 +284,9 @@ void main() { expect(parser.roots, [cached(wingspan)]); expect( resolution, - isA().having((r) => r.plugins, 'plugins', [wingspan]), + isA().having((r) => r.assets.plugins, 'plugins', [ + wingspan, + ]), ); }); @@ -299,7 +301,11 @@ void main() { expect( resolution, - isA().having((r) => r.plugins, 'plugins', vgvPlugins), + isA().having( + (r) => r.assets.plugins, + 'plugins', + vgvPlugins, + ), ); }); }); diff --git a/packages/vgv_ai_cli/test/src/translate/_assets.dart b/packages/vgv_ai_cli/test/src/translate/_assets.dart index d1b3335f..5c1edf54 100644 --- a/packages/vgv_ai_cli/test/src/translate/_assets.dart +++ b/packages/vgv_ai_cli/test/src/translate/_assets.dart @@ -73,11 +73,13 @@ PluginAssets pluginAssets({ List agents = const [], List mcpServers = const [], List hooks = const [], + List plugins = const [], }) => PluginAssets( skills: skills, agents: agents, mcpServers: mcpServers, hooks: hooks, + plugins: plugins, ); /// Parses a rendered `---` fenced frontmatter block back into its values. diff --git a/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart b/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart index 714c3c45..487e388e 100644 --- a/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/agents_vars_test.dart @@ -17,7 +17,6 @@ void main() { }) => AgentsVars( translators: translators, assets: assets ?? pluginAssets(), - plugins: vgvPlugins, skillsRoot: '/home/dev/.agents/skills', hooks: hooks, ).toMap(); diff --git a/packages/vgv_ai_cli/test/src/translate/translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translator_test.dart index 14a912eb..5e1e19c6 100644 --- a/packages/vgv_ai_cli/test/src/translate/translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translator_test.dart @@ -47,7 +47,6 @@ void main() { PluginAssets? using, }) => const Translator().translate( assets: using ?? assets, - plugins: vgvPlugins, home: home, selected: selected, ); diff --git a/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart index 2b1a571c..ffcbd5bd 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/antigravity_translator_test.dart @@ -56,7 +56,6 @@ void main() { ), ], ), - plugins: vgvPlugins, ); expect(jsonDecode(json), { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart index f78174a3..fc422bff 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/claude_translator_test.dart @@ -18,7 +18,7 @@ void main() { test('points Claude Code at the marketplace and enables both plugins', () { final settings = jsonDecode( - translator.settings(pluginAssets(), plugins: vgvPlugins), + translator.settings(pluginAssets(plugins: vgvPlugins)), ); expect(settings, { @@ -37,10 +37,10 @@ void main() { }); }); - test('enables only the plugins it is given', () { + test('enables only the plugins the assets came from', () { final wingspan = vgvPlugins.first; final settings = jsonDecode( - translator.settings(pluginAssets(), plugins: [wingspan]), + translator.settings(pluginAssets(plugins: [wingspan])), ) as Map; expect(settings['enabledPlugins'], { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart index eaaf8fdd..23e178f2 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/codex_translator_test.dart @@ -33,7 +33,6 @@ void main() { ), ], ), - plugins: vgvPlugins, ); expect(TomlDocument.parse(toml).toMap(), { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart index 45caedfa..0f7cfd96 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/copilot_translator_test.dart @@ -34,7 +34,6 @@ void main() { ), ], ), - plugins: vgvPlugins, ); expect(jsonDecode(json), { diff --git a/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart index 8a66e75d..3ecff067 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/gemini_translator_test.dart @@ -44,7 +44,6 @@ void main() { ), ], ), - plugins: vgvPlugins, ); expect( @@ -69,7 +68,6 @@ void main() { ), ], ), - plugins: vgvPlugins, ); expect( @@ -85,7 +83,7 @@ void main() { test('reads AGENTS.md without giving up its own GEMINI.md', () { expect( - jsonDecode(translator.settings(pluginAssets(), plugins: vgvPlugins)), + jsonDecode(translator.settings(pluginAssets())), containsPair('context', { 'fileName': ['AGENTS.md', 'GEMINI.md'], }), diff --git a/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart index 1c8db718..3709a9d2 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/opencode_translator_test.dart @@ -48,7 +48,6 @@ void main() { const RemoteMCPServer(name: 'open', url: 'https://open.example'), ], ), - plugins: vgvPlugins, ), ); diff --git a/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart b/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart index 3ef0a1bd..82a381e3 100644 --- a/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart +++ b/packages/vgv_ai_cli/test/src/translate/translators/pi_translator_test.dart @@ -15,7 +15,7 @@ void main() { }); test('has no settings, since Pi reads the shared skills natively', () { - expect(translator.settings(pluginAssets(), plugins: vgvPlugins), isNull); + expect(translator.settings(pluginAssets()), isNull); }); test('spells nothing else, since Pi reads prompts as written', () { From 5b83895057d648a52c94382cfefc1614f60d26a6 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Wed, 21 Oct 2026 14:46:23 +0200 Subject: [PATCH 3/8] fix: check only the plugins a home was installed from doctor resolved every pinned plugin, so after an install that named some of them it planned the rest as content that was missing and exited 70. It now resolves what the manifest records, and a plugin that was never installed is a note rather than an install to run. Also drop the PluginAssets copy that merge can do itself, record a distinct version in the skipped-plugin test so it can fail, and fix the install doc comment and the README example. --- .../lib/src/model/plugin_assets.dart | 18 ++++++---------- .../test/src/model/plugin_assets_test.dart | 21 +++++-------------- packages/vgv_ai_cli/README.md | 8 +++---- .../src/commands/doctor/doctor_command.dart | 8 ++++++- .../commands/doctor/model/plugin_version.dart | 4 +--- .../lib/src/commands/install_command.dart | 3 ++- .../lib/src/resolve/plugin_resolver.dart | 6 ++---- .../commands/doctor/doctor_command_test.dart | 16 +++++++++++++- .../doctor/model/plugin_version_test.dart | 2 +- .../src/commands/install_command_test.dart | 13 ++++++++++-- 10 files changed, 54 insertions(+), 45 deletions(-) diff --git a/packages/plugin_source/lib/src/model/plugin_assets.dart b/packages/plugin_source/lib/src/model/plugin_assets.dart index e948b857..48443aa8 100644 --- a/packages/plugin_source/lib/src/model/plugin_assets.dart +++ b/packages/plugin_source/lib/src/model/plugin_assets.dart @@ -24,28 +24,22 @@ class const PluginAssets({ /// itself names in its settings. final List plugins = const [], }) { - /// Combines what several plugins contribute into one [PluginAssets]. + /// Combines what the [plugins] contribute into one [PluginAssets]. /// /// Each skill, agent and MCP server is written to a path or key derived /// from its name, so the first declaration of a name wins and a later one /// is dropped rather than planned as a second write to the same place. - factory merge(Iterable assets) => PluginAssets( + factory merge( + Iterable assets, { + List plugins = const [], + }) => PluginAssets( skills: _firstByName([for (final a in assets) ...a.skills], (s) => s.name), agents: _firstByName([for (final a in assets) ...a.agents], (a) => a.name), mcpServers: _firstByName([ for (final a in assets) ...a.mcpServers, ], (s) => s.name), hooks: [for (final a in assets) ...a.hooks], - plugins: [for (final a in assets) ...a.plugins], - ); - - /// These assets, with [plugin] credited for them. - PluginAssets withPlugin(PluginRef plugin) => PluginAssets( - skills: skills, - agents: agents, - mcpServers: mcpServers, - hooks: hooks, - plugins: [...plugins, plugin], + plugins: plugins, ); /// The skills a user can invoke as a command. diff --git a/packages/plugin_source/test/src/model/plugin_assets_test.dart b/packages/plugin_source/test/src/model/plugin_assets_test.dart index a1b9a18c..f84085a3 100644 --- a/packages/plugin_source/test/src/model/plugin_assets_test.dart +++ b/packages/plugin_source/test/src/model/plugin_assets_test.dart @@ -45,17 +45,6 @@ void main() { setUp(() => fs = MemoryFileSystem.test()); - group('withPlugin', () { - test('credits the plugin for what it contributes', () { - final assets = pluginAssets(skills: [skill('green-gate')]); - - final credited = assets.withPlugin(wingspan); - - expect(credited.plugins, [wingspan]); - expect(credited.skills, assets.skills); - }); - }); - group('invocableSkills', () { test('keeps only the skills a user can invoke as a command', () { final assets = pluginAssets( @@ -82,11 +71,11 @@ void main() { }); group('merge', () { - test('names every plugin that contributed', () { - final merged = PluginAssets.merge([ - pluginAssets().withPlugin(wingspan), - pluginAssets().withPlugin(flutter), - ]); + test('names the plugins the assets came from', () { + final merged = PluginAssets.merge( + [pluginAssets(), pluginAssets()], + plugins: [wingspan, flutter], + ); expect(merged.plugins, [wingspan, flutter]); }); diff --git a/packages/vgv_ai_cli/README.md b/packages/vgv_ai_cli/README.md index f3b544fe..296461f1 100644 --- a/packages/vgv_ai_cli/README.md +++ b/packages/vgv_ai_cli/README.md @@ -66,7 +66,7 @@ vgv_ai install --no-claude # Also overwrite your values that conflict with the ones install writes vgv_ai install --force -# Install just one of the pinned VGV plugins +# Install just the VGV plugins you name, instead of every pinned one vgv_ai install --plugin vgv-wingspan ``` @@ -85,9 +85,9 @@ how `doctor` and `uninstall` tell what's yours from what's VGV's. are written whole. A file you edited since `install` wrote it, or one at the same path that `install` didn't write, is kept unless you pass `--force`. - **Selection:** `--only` and `--no-` can't be combined. -- **Plugins:** without `--plugin`, every pinned plugin is installed. Naming one - leaves what a previous `install` wrote for the others in place; `uninstall` - removes that. +- **Plugins:** without `--plugin`, every pinned plugin is installed. Pass it + once per plugin to name the ones you want. What a previous `install` wrote + for the plugins you leave out stays where it is; `uninstall` removes that. ### doctor diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index 0081940d..0cc963ff 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -166,7 +166,13 @@ class DoctorCommand({ Set selected, ) async { final progress = _logger.progress('Resolving plugins'); - switch (await _resolver.resolve(home)) { + // Only the plugins the home was installed from, so a plugin left out of + // `install --plugin` is not reported as content that is missing. + final recorded = manifest.plugins.keys.toSet(); + switch (await _resolver.resolve( + home, + only: recorded.isEmpty ? null : recorded, + )) { case ResolvedAssets(:final assets): progress.complete('Resolved plugins'); final translation = await _translator.translate( diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart index 72302786..52a67e96 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/plugin_version.dart @@ -67,9 +67,7 @@ class const PluginVersion._({ PluginStatus.newer => Finding.stale( '$name $installed installed, newer than the pinned $pinned', ), - PluginStatus.unrecorded => Finding.suggestion( - '$name $pinned not installed', - ), + PluginStatus.unrecorded => Finding.warning('$name $pinned not installed'), }; } } diff --git a/packages/vgv_ai_cli/lib/src/commands/install_command.dart b/packages/vgv_ai_cli/lib/src/commands/install_command.dart index ed5c310f..f4de201b 100644 --- a/packages/vgv_ai_cli/lib/src/commands/install_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/install_command.dart @@ -216,7 +216,8 @@ class InstallCommand({ }; } - /// Resolves and parses every pinned VGV plugin into one asset bundle. + /// Resolves and parses the plugins `--plugin` selects into one asset + /// bundle, or every pinned one when it selects none. /// /// Returns `null`, having said why, when any plugin cannot be resolved or /// parsed. diff --git a/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart b/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart index 151a2e47..33babc25 100644 --- a/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart +++ b/packages/vgv_ai_cli/lib/src/resolve/plugin_resolver.dart @@ -47,7 +47,7 @@ class const PluginResolver({ } } - return ResolvedAssets(assets: PluginAssets.merge(parsed)); + return ResolvedAssets(assets: PluginAssets.merge(parsed, plugins: plugins)); } /// [plugin] parsed from the first of [sources] holding a copy that parses. @@ -67,9 +67,7 @@ class const PluginResolver({ switch (await source.resolve(plugin)) { case ResolvedPlugin(:final directory): try { - return ResolvedAssets( - assets: _parser.parse(directory).withPlugin(plugin), - ); + return ResolvedAssets(assets: _parser.parse(directory)); } on PluginParseException catch (error) { unparseable = UnparseableAssets(plugin, error); } diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index a015d3f9..baa4fdac 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -37,10 +37,12 @@ class _FakeResolver extends Fake implements PluginResolver { ); int calls = 0; + Set? only; @override Future resolve(Directory home, {Set? only}) async { calls++; + this.only = only; return resolution; } } @@ -679,6 +681,16 @@ void main() { expect(exitCode, ExitCode.success.code); verifyInfo(' ${wingspan.name} ${wingspan.version} not installed'); + // Nothing was recorded, so every pinned plugin is still checked. + expect(resolver.only, isNull); + }); + + test('only the recorded plugins are checked', () async { + installWith({wingspan.name: wingspan.version}); + + await runDoctor(); + + expect(resolver.only, {wingspan.name}); }); test( @@ -689,7 +701,9 @@ void main() { owned('.gemini/agents/a.md', harness: Harness.gemini), owned('.pi/agent/a.md', harness: Harness.pi), ]); - store.save(store.load()!.withPlugins({})); + store.save( + store.load()!.withPlugins({...pinned, wingspan.name: '0.0.1'}), + ); await runDoctor(args: ['--no-pi']); diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart index 1eaaba40..e23b89cc 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/model/plugin_version_test.dart @@ -63,7 +63,7 @@ void main() { expect(version.status, PluginStatus.unrecorded); expect( version.finding, - isFinding(FindingKind.suggestion, 'wingspan 0.2.0 not installed'), + isFinding(FindingKind.warning, 'wingspan 0.2.0 not installed'), ); }); }); diff --git a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart index a7873108..76b25a8b 100644 --- a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart @@ -275,7 +275,15 @@ void main() { }); test('keeps the recorded version of a plugin it skips', () async { + final skipped = vgvPlugins.last; await runInstall(['--only', 'claude']); + final store = ManifestStore(home); + store.save( + store.load()!.withPlugins({ + for (final plugin in vgvPlugins) plugin.name: plugin.version, + skipped.name: '0.0.1', + }), + ); await runInstall([ '--only', @@ -284,8 +292,9 @@ void main() { vgvPlugins.first.name, ]); - expect(ManifestStore(home).load()!.plugins, { - for (final plugin in vgvPlugins) plugin.name: plugin.version, + expect(store.load()!.plugins, { + vgvPlugins.first.name: vgvPlugins.first.version, + skipped.name: '0.0.1', }); }); From c705bee48ac04179cf5d88ccb7edc74caa54595a Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Thu, 22 Oct 2026 10:30:07 +0200 Subject: [PATCH 4/8] feat: record the plugin versions per harness install --plugin can leave a plugin out of one harness while another is installed from it, so a manifest that records one set for the whole home makes doctor plan both for both. It now records them per harness, plans each harness against the plugins it was installed from, and reports the versions in that harness's own section. The fix doctor prints names those plugins too, so following it cannot install one that was deliberately left out, and no combined fix is printed when the harnesses need different plugins. Manifest version 3; an older one reads as empty, as a malformed one does. --- packages/vgv_ai_cli/README.md | 8 +- .../src/commands/doctor/doctor_command.dart | 146 ++++++++++-------- .../commands/doctor/model/fix_command.dart | 21 ++- .../commands/doctor/model/report_section.dart | 4 + .../lib/src/commands/install_command.dart | 22 ++- .../lib/src/manifest/manifest_auditor.dart | 10 +- .../lib/src/model/install_manifest.dart | 60 ++++--- .../commands/doctor/doctor_command_test.dart | 110 +++++++++---- .../doctor/model/fix_command_test.dart | 44 ++++++ .../src/commands/install_command_test.dart | 28 +++- .../src/manifest/manifest_auditor_test.dart | 28 +++- .../test/src/model/install_manifest_test.dart | 90 ++++++++--- 12 files changed, 411 insertions(+), 160 deletions(-) diff --git a/packages/vgv_ai_cli/README.md b/packages/vgv_ai_cli/README.md index 296461f1..08756c41 100644 --- a/packages/vgv_ai_cli/README.md +++ b/packages/vgv_ai_cli/README.md @@ -106,10 +106,10 @@ vgv_ai doctor --no-claude For each harness, `doctor` reports the files and keys that are missing, were changed since, or differ from what this version installs. It also compares the -plugin versions you installed from with the ones this version pins, notes a -pinned plugin you haven't installed, suggests configuring a harness that's -installed but not configured, and warns about a configured harness that's no -longer installed. +plugin versions each harness was installed from with the ones this version +pins, notes a pinned plugin a harness wasn't installed from, suggests +configuring a harness that's installed but not configured, and warns about a +configured harness that's no longer installed. Every problem comes with the `install` command that fixes it, and the report ends with one command that fixes everything `install` can, such as diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index 0cc963ff..aff9c289 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -160,36 +160,58 @@ class DoctorCommand({ /// What a dry run of `install` for [selected] would do, or an unchecked /// plan, having said why, when the plugins cannot be resolved. + /// + /// Harnesses installed from different plugins are planned apart, so a + /// plugin one of them was installed without is not planned for it. Future _plan( InstallManifest manifest, Directory home, Set selected, ) async { final progress = _logger.progress('Resolving plugins'); - // Only the plugins the home was installed from, so a plugin left out of - // `install --plugin` is not reported as content that is missing. - final recorded = manifest.plugins.keys.toSet(); - switch (await _resolver.resolve( - home, - only: recorded.isEmpty ? null : recorded, - )) { - case ResolvedAssets(:final assets): - progress.complete('Resolved plugins'); - final translation = await _translator.translate( - assets: assets, - home: home, - selected: selected, - ); - return ContentPlan( - _reconciler - .apply(translation.changes, manifest: manifest, dryRun: true) - .changes, - ); - case UnavailableAssets(:final plugin, :final reasons): - return _unchecked(progress, 'resolve $plugin', reasons); - case UnparseableAssets(:final plugin, :final error): - return _unchecked(progress, 'parse $plugin', ['$error']); + final changes = []; + + for (final MapEntry(key: names, value: harnesses) in _byPlugins( + manifest, + selected, + ).entries) { + final only = names.isEmpty ? null : names.split(' ').toSet(); + switch (await _resolver.resolve(home, only: only)) { + case ResolvedAssets(:final assets): + final translation = await _translator.translate( + assets: assets, + home: home, + selected: harnesses, + ); + changes.addAll( + _reconciler + .apply(translation.changes, manifest: manifest, dryRun: true) + .changes, + ); + case UnavailableAssets(:final plugin, :final reasons): + return _unchecked(progress, 'resolve $plugin', reasons); + case UnparseableAssets(:final plugin, :final error): + return _unchecked(progress, 'parse $plugin', ['$error']); + } + } + + progress.complete('Resolved plugins'); + return ContentPlan(changes); + } + + /// [selected] grouped by the plugins each harness was installed from, + /// keyed by their names in order, and by the empty string for a harness + /// the manifest records none for, which is checked against every pin. + Map> _byPlugins( + InstallManifest manifest, + Set selected, + ) { + final groups = >{}; + for (final harness in selected) { + final recorded = manifest.plugins[harness]?.keys.toList() ?? []; + groups.putIfAbsent((recorded..sort()).join(' '), () => {}).add(harness); } + return groups; } /// An unchecked plan, having failed [progress] and listed [reasons] for @@ -211,10 +233,10 @@ class DoctorCommand({ ); } - /// The sections of the report: the plugin versions, and why content was - /// not checked if it was not, when a harness is [installed], the shared - /// skills when an [installed] harness reads them, and each harness that - /// is [installed] or [detected]. + /// The sections of the report: why content was not checked if it was not, + /// the shared skills when an [installed] harness reads them, and each + /// harness that is [installed] or [detected], with the plugin versions it + /// was installed from. /// /// [installed] are the harnesses of the selected scope VGV configured, and /// [detected] those of it on this machine. @@ -228,7 +250,12 @@ class DoctorCommand({ final readsShared = installed.intersection(_readShared); return [ - if (installed.isNotEmpty) _pluginsSection(manifest, installed, plan), + if (plan.skipped case final skipped?) + ReportSection( + title: 'Plugins', + installs: installed, + findings: [skipped], + ), if (readsShared.isNotEmpty && manifest.entriesFor(null).isNotEmpty) ReportSection( title: 'Shared', @@ -248,25 +275,6 @@ class DoctorCommand({ ]; } - /// The plugin versions [installed] read, and why content was not checked - /// if [plan] was not. - ReportSection _pluginsSection( - InstallManifest manifest, - Set installed, - ContentPlan plan, - ) => ReportSection( - title: 'Plugins', - installs: installed, - findings: [ - for (final plugin in vgvPlugins) - PluginVersion.of( - plugin, - installed: manifest.plugins[plugin.name], - ).finding, - ?plan.skipped, - ], - ); - /// The section for [harness], which VGV configured when [isInstalled] and /// is on this machine when [isDetected]. ReportSection _harnessSection( @@ -276,21 +284,35 @@ class DoctorCommand({ ContentPlan plan, { required bool isInstalled, required bool isDetected, - }) => ReportSection( - title: harness.displayName, - installs: {harness}, - findings: [ - if (!isDetected) - const Finding.warning( - 'not installed on this machine, but VGV AI CLI configured it', - ), - if (!isInstalled) - const Finding.suggestion( - "installed, but VGV AI CLI hasn't configured it", - ), - ..._entryFindings(harness, manifest, home, plan), - ], - ); + }) { + final recorded = manifest.plugins[harness] ?? const {}; + return ReportSection( + title: harness.displayName, + installs: {harness}, + plugins: _selected(recorded), + findings: [ + if (!isDetected) + const Finding.warning( + 'not installed on this machine, but VGV AI CLI configured it', + ), + if (!isInstalled) + const Finding.suggestion( + "installed, but VGV AI CLI hasn't configured it", + ), + if (isInstalled) + for (final plugin in vgvPlugins) + PluginVersion.of(plugin, installed: recorded[plugin.name]).finding, + ..._entryFindings(harness, manifest, home, plan), + ], + ); + } + + /// The plugin names a fix must name, or none when [recorded] holds every + /// pinned plugin, which `install` writes without being asked. + Set _selected(Map recorded) => + vgvPlugins.every((plugin) => recorded.containsKey(plugin.name)) + ? const {} + : recorded.keys.toSet(); /// What is wrong with the entries [owner] has in [manifest], and the /// files [plan] would add for it. diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart index 1edb0c56..b2d736e7 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart @@ -1,3 +1,5 @@ +import 'package:collection/collection.dart'; +import 'package:plugin_source/plugin_source.dart'; import 'package:vgv_ai_cli/src/model/model.dart'; /// {@template fix_command} @@ -9,15 +11,28 @@ class const FixCommand({ /// Whether VGV's values must replace ones changed since install. final bool force = false, + + /// The plugins to install, or empty for every pinned one. + /// + /// Named so the fix cannot install a plugin the user left out of a + /// harness with `install --plugin`. + final Set plugins = const {}, }) { /// One command that does what every one of [fixes] does, or null when - /// there are none. + /// there are none, or when they install different plugins and so cannot + /// be said in one command. static FixCommand? combine(Iterable fixes) { - final all = fixes.nonNulls; + final all = fixes.nonNulls.toList(); if (all.isEmpty) return null; + + final plugins = all.first.plugins; + const equality = SetEquality(); + if (all.any((fix) => !equality.equals(fix.plugins, plugins))) return null; + return FixCommand( harnesses: {for (final fix in all) ...fix.harnesses}, force: all.any((fix) => fix.force), + plugins: plugins, ); } @@ -26,6 +41,8 @@ class const FixCommand({ 'vgv_ai install', for (final harness in Harness.values) if (harnesses.contains(harness)) '--only ${harness.name}', + for (final plugin in vgvPlugins) + if (plugins.contains(plugin.name)) '--plugin ${plugin.name}', if (force) '--force', ].join(' '); } diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart index 4c57405e..58a990cb 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart @@ -15,6 +15,9 @@ class const ReportSection({ /// What was found, in the order it was found. required final List findings, + + /// The plugins the fix installs, or empty for every pinned one. + final Set plugins = const {}, }) { /// Whether anything was found that is wrong. bool get hasProblems => findings.any((finding) => finding.isProblem); @@ -28,6 +31,7 @@ class const ReportSection({ ? FixCommand( harnesses: installs, force: findings.any((finding) => finding.needsForce), + plugins: plugins, ) : null; } diff --git a/packages/vgv_ai_cli/lib/src/commands/install_command.dart b/packages/vgv_ai_cli/lib/src/commands/install_command.dart index f4de201b..2b6dc53a 100644 --- a/packages/vgv_ai_cli/lib/src/commands/install_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/install_command.dart @@ -143,7 +143,7 @@ class InstallCommand({ store.save( from: manifest, to: reconciliation.manifest.withPlugins( - _pluginVersions(manifest, resolved.assets.plugins), + _pluginVersions(manifest, resolved.assets.plugins, selected), ), ); final anyChangeFailed = applied.any( @@ -200,19 +200,27 @@ class InstallCommand({ return names.isEmpty ? null : names.toSet(); } - /// The versions to record for the pinned plugins: [installed] for the ones - /// this run wrote, and what [manifest] already records for the ones it - /// skipped, whose files are still installed. - Map _pluginVersions( + /// The versions to record for each harness of [selected]: [installed] for + /// the plugins this run wrote, and what [manifest] already records for the + /// ones it skipped, whose files are still installed. Every other harness + /// keeps what it had. + Map> _pluginVersions( InstallManifest manifest, List installed, + Set selected, ) { final written = { for (final plugin in installed) plugin.name: plugin.version, }; return { - for (final plugin in vgvPlugins) - plugin.name: ?(written[plugin.name] ?? manifest.plugins[plugin.name]), + ...manifest.plugins, + for (final harness in selected) + harness: { + for (final plugin in vgvPlugins) + plugin.name: + ?(written[plugin.name] ?? + manifest.plugins[harness]?[plugin.name]), + }, }; } diff --git a/packages/vgv_ai_cli/lib/src/manifest/manifest_auditor.dart b/packages/vgv_ai_cli/lib/src/manifest/manifest_auditor.dart index a92cb56e..49da2137 100644 --- a/packages/vgv_ai_cli/lib/src/manifest/manifest_auditor.dart +++ b/packages/vgv_ai_cli/lib/src/manifest/manifest_auditor.dart @@ -73,12 +73,14 @@ class const ManifestAuditor({ final narrowed = [ for (final ReversedEntry(:retained) in reversed) ?retained, ]; + final kept = [...manifest.without(settled).entries, ...narrowed]; + final remaining = {for (final entry in kept) entry.owner}; return Reversal( entries: reversed, - manifest: InstallManifest([ - ...manifest.without(settled).entries, - ...narrowed, - ], manifest.plugins), + manifest: InstallManifest(kept, { + for (final MapEntry(:key, :value) in manifest.plugins.entries) + if (remaining.contains(key)) key: value, + }), ); } diff --git a/packages/vgv_ai_cli/lib/src/model/install_manifest.dart b/packages/vgv_ai_cli/lib/src/model/install_manifest.dart index c19b0874..47b19695 100644 --- a/packages/vgv_ai_cli/lib/src/model/install_manifest.dart +++ b/packages/vgv_ai_cli/lib/src/model/install_manifest.dart @@ -174,7 +174,7 @@ final class const MarkdownSections({ /// {@template install_manifest} /// Everything VGV wrote under a home directory, by file, and the plugin -/// versions it was written from. +/// versions each harness was written from. /// /// `install` writes it, and `doctor` and `uninstall` read it, so this is the /// one place ownership is defined. It is persisted rather than recomputed @@ -196,9 +196,10 @@ class InstallManifest extends Equatable { /// /// A malformed manifest is empty: an unrecorded file is the user's, which /// is the safe way to be wrong. For the same reason, an entry whose path - /// is absolute or leads out of [home] is dropped. A version 1 manifest is - /// read as one that records no plugin versions. One from a newer version - /// is null, because writing over it would lose everything it records. + /// is absolute or leads out of [home] is dropped, and so is a manifest + /// from an older version, which recorded its plugin versions for the whole + /// home rather than per harness. One from a newer version is null, because + /// writing over it would lose everything it records. static InstallManifest? decode(String source, {required String home}) { final Object? json; try { @@ -209,25 +210,33 @@ class InstallManifest extends Equatable { return switch (json) { { - 'version': 1 || InstallManifest.version, + 'version': InstallManifest.version, 'entries': final List list, } => - InstallManifest( - [ - for (final item in list) - ?ManifestEntry.fromJson(_resolved(item, home: home)), - ], - { - if (json['plugins'] case final Map plugins) - for (final MapEntry(:key, :value) in plugins.entries) - if (value is String) key: value, - }, - ), + InstallManifest([ + for (final item in list) + ?ManifestEntry.fromJson(_resolved(item, home: home)), + ], _plugins(json['plugins'])), {'version': final int other} when other > version => null, _ => InstallManifest(), }; } + /// The versions [json] records per harness, without what this version + /// cannot read. + static Map> _plugins(Object? json) { + if (json is! Map) return const {}; + return { + for (final MapEntry(:key, :value) in json.entries) + if (Harness.values.asNameMap()[key] case final harness?) + if (value is Map) + harness: { + for (final MapEntry(:key, :value) in value.entries) + if (value is String) key: value, + }, + }; + } + /// [item] with its path resolved against [home], or null when the path /// is absolute or leads out of [home]. static Object? _resolved(Object? item, {required String home}) { @@ -241,12 +250,16 @@ class InstallManifest extends Equatable { } /// The format [encode] writes. - static const version = 2; + static const version = 3; final Map _entries; - /// Each plugin's name to the version VGV's files were written from. - final Map plugins; + /// Each harness to the plugin versions its files were written from, by + /// plugin name. + /// + /// Per harness rather than per home, since `install --plugin` can leave a + /// plugin out of one harness while another is installed from it. + final Map> plugins; /// Every entry, in the order they were recorded. Iterable get entries => _entries.values; @@ -279,8 +292,9 @@ class InstallManifest extends Equatable { ); } - /// This manifest with [plugins] as the versions it was written from. - InstallManifest withPlugins(Map plugins) { + /// This manifest with [plugins] as the versions each harness was written + /// from. + InstallManifest withPlugins(Map> plugins) { return InstallManifest(entries, plugins); } @@ -289,7 +303,9 @@ class InstallManifest extends Equatable { String encode({required String home}) { final json = { 'version': version, - 'plugins': plugins, + 'plugins': { + for (final MapEntry(:key, :value) in plugins.entries) key.name: value, + }, 'entries': [ for (final entry in entries) {...entry.toJson(), 'path': p.relative(entry.path, from: home)}, diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index baa4fdac..d3216494 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -37,12 +37,14 @@ class _FakeResolver extends Fake implements PluginResolver { ); int calls = 0; - Set? only; + + /// The plugins each call asked for, in order. + final resolved = ?>[]; @override Future resolve(Directory home, {Set? only}) async { calls++; - this.only = only; + resolved.add(only); return resolution; } } @@ -149,14 +151,18 @@ void main() { }; /// Installs [changes] with the real reconciler, recording the pinned - /// plugin versions, as `install` does, and plans them again, so they - /// are current unless a test changes the plan. + /// plugin versions for each harness it writes for, as `install` does, + /// and plans them again, so they are current unless a test changes the + /// plan. void install(List changes) { + final manifest = const Reconciler() + .apply(changes, manifest: store.load()!) + .manifest; store.save( - const Reconciler() - .apply(changes, manifest: store.load()!) - .manifest - .withPlugins(pinned), + manifest.withPlugins({ + ...manifest.plugins, + for (final change in changes) ?change.harness: pinned, + }), ); translator.changes.addAll(changes); } @@ -195,12 +201,11 @@ void main() { final exitCode = await runDoctor(); expect(exitCode, ExitCode.success.code); - verifyInfo('✓ Plugins'); + verifyInfo('✓ Shared'); + verifyInfo('✓ Codex'); for (final plugin in vgvPlugins) { verifyInfo(' ${plugin.name} ${plugin.version}'); } - verifyInfo('✓ Shared'); - verifyInfo('✓ Codex'); }); test('reports nothing installed when there is no manifest', () async { @@ -645,7 +650,7 @@ void main() { void installWith(Map plugins) { install([owned('.codex/agents/a.toml', harness: Harness.codex)]); - store.save(store.load()!.withPlugins(plugins)); + store.save(store.load()!.withPlugins({Harness.codex: plugins})); } test('an older recorded version exits 70 with the install fix', () async { @@ -654,7 +659,7 @@ void main() { final exitCode = await runDoctor(); expect(exitCode, ExitCode.software.code); - verifyInfo('✗ Plugins'); + verifyInfo('✗ Codex'); verifyInfo( ' ${wingspan.name} 0.0.1 installed, ${wingspan.version} pinned', ); @@ -682,43 +687,96 @@ void main() { expect(exitCode, ExitCode.success.code); verifyInfo(' ${wingspan.name} ${wingspan.version} not installed'); // Nothing was recorded, so every pinned plugin is still checked. - expect(resolver.only, isNull); + expect(resolver.resolved, [null]); }); - test('only the recorded plugins are checked', () async { - installWith({wingspan.name: wingspan.version}); + test( + 'only the plugins a harness was installed from are checked', + () async { + installWith({wingspan.name: wingspan.version}); - await runDoctor(); + await runDoctor(); - expect(resolver.only, {wingspan.name}); - }); + expect(resolver.resolved, [ + {wingspan.name}, + ]); + }, + ); test( - 'are fixed by installing every configured harness in scope', + 'harnesses installed from different plugins are planned apart', () async { install([ owned('.codex/agents/a.toml', harness: Harness.codex), owned('.gemini/agents/a.md', harness: Harness.gemini), - owned('.pi/agent/a.md', harness: Harness.pi), ]); store.save( - store.load()!.withPlugins({...pinned, wingspan.name: '0.0.1'}), + store.load()!.withPlugins({ + Harness.codex: {wingspan.name: wingspan.version}, + Harness.gemini: pinned, + }), + ); + + await runDoctor(); + + expect( + resolver.resolved, + unorderedEquals([ + {wingspan.name}, + {for (final plugin in vgvPlugins) plugin.name}, + ]), + ); + }, + ); + + test( + 'the fix names the plugins the harness was installed from', + () async { + install([owned('.codex/agents/a.toml', harness: Harness.codex)]); + store.save( + store.load()!.withPlugins({ + Harness.codex: {wingspan.name: '0.0.1'}, + }), ); - await runDoctor(args: ['--no-pi']); + await runDoctor(); - verifyInfo(' Run vgv_ai install --only gemini --only codex'); + verifyInfo( + ' Run vgv_ai install --only codex --plugin ${wingspan.name}', + ); }, ); - test('are not reported when nothing in scope is configured', () async { + test('are reported under each harness they were installed for', () async { + install([ + owned('.codex/agents/a.toml', harness: Harness.codex), + owned('.gemini/agents/a.md', harness: Harness.gemini), + ]); + store.save( + store.load()!.withPlugins({ + Harness.codex: {...pinned, wingspan.name: '0.0.1'}, + Harness.gemini: pinned, + }), + ); + + await runDoctor(); + + verifyInfo('✗ Codex'); + verifyInfo( + ' ${wingspan.name} 0.0.1 installed, ${wingspan.version} pinned', + ); + verifyInfo(' Run vgv_ai install --only codex'); + verifyInfo('✓ Gemini'); + }); + + test('are not reported for a harness VGV has not configured', () async { installWith({}); detect(const [Harness.codex, Harness.pi]); final exitCode = await runDoctor(args: ['--only', 'pi']); expect(exitCode, ExitCode.success.code); - verifyNever(() => logger.info(any(that: endsWith('Plugins')))); + verifyNever(() => logger.info(any(that: contains(wingspan.name)))); }); }); diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/model/fix_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/model/fix_command_test.dart index 92a1421b..66b4ea86 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/model/fix_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/model/fix_command_test.dart @@ -1,3 +1,4 @@ +import 'package:plugin_source/plugin_source.dart'; import 'package:test/test.dart'; import 'package:vgv_ai_cli/src/commands/doctor/model/model.dart'; import 'package:vgv_ai_cli/src/model/model.dart'; @@ -18,6 +19,19 @@ void main() { ); }); + test('names each plugin with --plugin, in pinned order', () { + expect( + FixCommand( + harnesses: const {Harness.codex}, + plugins: {for (final plugin in vgvPlugins) plugin.name}, + ).toString(), + [ + 'vgv_ai install --only codex', + for (final plugin in vgvPlugins) '--plugin ${plugin.name}', + ].join(' '), + ); + }); + group('combine', () { test('installs every harness any fix names', () { expect( @@ -40,6 +54,36 @@ void main() { ); }); + test('keeps the plugins every fix installs', () { + expect( + FixCommand.combine([ + FixCommand( + harnesses: const {Harness.codex}, + plugins: {vgvPlugins.first.name}, + ), + FixCommand( + harnesses: const {Harness.pi}, + plugins: {vgvPlugins.first.name}, + ), + ]).toString(), + 'vgv_ai install --only codex --only pi ' + '--plugin ${vgvPlugins.first.name}', + ); + }); + + test('is null when the fixes install different plugins', () { + expect( + FixCommand.combine([ + FixCommand( + harnesses: const {Harness.codex}, + plugins: {vgvPlugins.first.name}, + ), + const FixCommand(harnesses: {Harness.pi}), + ]), + isNull, + ); + }); + test('is null when there is nothing to fix', () { expect(FixCommand.combine(const []), isNull); expect(FixCommand.combine(const [null, null]), isNull); diff --git a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart index 76b25a8b..81c39dea 100644 --- a/packages/vgv_ai_cli/test/src/commands/install_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/install_command_test.dart @@ -270,7 +270,7 @@ void main() { expect(exitCode, ExitCode.success.code); expect(translator.assets?.plugins, [vgvPlugins.first]); expect(ManifestStore(home).load()!.plugins, { - vgvPlugins.first.name: vgvPlugins.first.version, + Harness.claude: {vgvPlugins.first.name: vgvPlugins.first.version}, }); }); @@ -280,8 +280,10 @@ void main() { final store = ManifestStore(home); store.save( store.load()!.withPlugins({ - for (final plugin in vgvPlugins) plugin.name: plugin.version, - skipped.name: '0.0.1', + Harness.claude: { + for (final plugin in vgvPlugins) plugin.name: plugin.version, + skipped.name: '0.0.1', + }, }), ); @@ -293,8 +295,10 @@ void main() { ]); expect(store.load()!.plugins, { - vgvPlugins.first.name: vgvPlugins.first.version, - skipped.name: '0.0.1', + Harness.claude: { + vgvPlugins.first.name: vgvPlugins.first.version, + skipped.name: '0.0.1', + }, }); }); @@ -583,7 +587,9 @@ void main() { final manifest = ManifestStore(home).load()!; expect(manifest[homeFile('.claude/a.md').path], isA()); expect(manifest.plugins, { - for (final plugin in vgvPlugins) plugin.name: plugin.version, + Harness.claude: { + for (final plugin in vgvPlugins) plugin.name: plugin.version, + }, }); }); @@ -591,12 +597,18 @@ void main() { planOne(); await runInstall(['--only', 'claude']); final store = ManifestStore(home); - store.save(store.load()!.withPlugins(const {'vgv-wingspan': '0.0.1'})); + store.save( + store.load()!.withPlugins(const { + Harness.claude: {'vgv-wingspan': '0.0.1'}, + }), + ); await runInstall(['--only', 'claude']); expect(store.load()!.plugins, { - for (final plugin in vgvPlugins) plugin.name: plugin.version, + Harness.claude: { + for (final plugin in vgvPlugins) plugin.name: plugin.version, + }, }); }); diff --git a/packages/vgv_ai_cli/test/src/manifest/manifest_auditor_test.dart b/packages/vgv_ai_cli/test/src/manifest/manifest_auditor_test.dart index 3d786ee6..3883b16f 100644 --- a/packages/vgv_ai_cli/test/src/manifest/manifest_auditor_test.dart +++ b/packages/vgv_ai_cli/test/src/manifest/manifest_auditor_test.dart @@ -623,13 +623,35 @@ void main() { expect(manifest.file.readAsStringSync(), before); }); - test('keeps the plugin versions it records', () { + test('keeps the plugin versions of a harness it leaves alone', () { + const recorded = { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }; + install([ + owned(seed('.codex/a.md', 'a'), 'a', harness: Harness.codex), + ]); + + final result = auditor.reverse( + manifest.load()!.withPlugins(recorded), + fs, + owners: const {Harness.claude}, + ); + + expect(result.manifest.plugins, recorded); + }); + + test('forgets the plugin versions of a harness it empties', () { + install([owned(file('.codex/a.md'), 'a', harness: Harness.codex)]); + final result = auditor.reverse( - InstallManifest(const [], const {'vgv-wingspan': '0.0.5'}), + manifest.load()!.withPlugins(const { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }), fs, ); - expect(result.manifest.plugins, {'vgv-wingspan': '0.0.5'}); + expect(result.manifest.entries, isEmpty); + expect(result.manifest.plugins, isEmpty); }); test('does nothing with an empty manifest', () { diff --git a/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart b/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart index e1dee865..779550e8 100644 --- a/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart +++ b/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart @@ -44,7 +44,10 @@ void main() { test('round trips every kind of entry, and the plugin versions', () { final manifest = InstallManifest( const [owned, merged, sections], - const {'vgv-wingspan': '0.0.5', 'vgv-ai-flutter-plugin': '0.0.6'}, + const { + Harness.claude: {'vgv-wingspan': '0.0.5'}, + Harness.codex: {'vgv-ai-flutter-plugin': '0.0.6'}, + }, ); final decoded = InstallManifest.decode( @@ -79,8 +82,8 @@ void main() { isA().having((e) => e.sha256, 'sha256', 'abc'), ); expect(decoded.plugins, { - 'vgv-wingspan': '0.0.5', - 'vgv-ai-flutter-plugin': '0.0.6', + Harness.claude: {'vgv-wingspan': '0.0.5'}, + Harness.codex: {'vgv-ai-flutter-plugin': '0.0.6'}, }); }); @@ -88,12 +91,16 @@ void main() { final json = jsonDecode( InstallManifest( const [owned, merged], - const {'vgv-wingspan': '0.0.5'}, + const { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }, ).encode(home: home), ) as Map; - expect(json['version'], 2); - expect(json['plugins'], {'vgv-wingspan': '0.0.5'}); + expect(json['version'], 3); + expect(json['plugins'], { + 'codex': {'vgv-wingspan': '0.0.5'}, + }); expect(json['entries'], [ { 'path': '.agents/skills/a/SKILL.md', @@ -139,9 +146,20 @@ void main() { expect(decode(null).plugins, isEmpty); expect(decode(['vgv-wingspan']).plugins, isEmpty); + expect(decode({'codex': 'vgv-wingspan'}).plugins, isEmpty); + expect( + decode({ + 'nope': {'vgv-wingspan': '0.0.5'}, + }).plugins, + isEmpty, + ); expect( - decode({'vgv-wingspan': 5, 'vgv-ai-flutter-plugin': '0.0.5'}).plugins, - {'vgv-ai-flutter-plugin': '0.0.5'}, + decode({ + 'codex': {'vgv-wingspan': 5, 'vgv-ai-flutter-plugin': '0.0.5'}, + }).plugins, + { + Harness.codex: {'vgv-ai-flutter-plugin': '0.0.5'}, + }, ); }); @@ -165,16 +183,17 @@ void main() { ); }); - test('reads a version 1 manifest as one with no plugin versions', () { + test('is empty for the version before this one', () { final manifest = InstallManifest.decode( jsonEncode({ - 'version': 1, + 'version': InstallManifest.version - 1, 'entries': [onDisk(owned)], + 'plugins': {'vgv-wingspan': '0.0.5'}, }), home: home, )!; - expect(manifest.entries.map((e) => e.path), [owned.path]); + expect(manifest.entries, isEmpty); expect(manifest.plugins, isEmpty); }); @@ -248,31 +267,46 @@ void main() { const updated = OwnedFile(path: '/x', owner: null, sha256: 'new'); final manifest = InstallManifest( const [OwnedFile(path: '/x', owner: null, sha256: 'old'), merged], - const {'vgv-wingspan': '0.0.5'}, + const { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }, ).record(updated); expect(manifest.entries, [updated, merged]); - expect(manifest.plugins, {'vgv-wingspan': '0.0.5'}); + expect(manifest.plugins, { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }); }); test('drops the entries for the given paths', () { final manifest = InstallManifest( const [owned, merged, sections], - const {'vgv-wingspan': '0.0.5'}, + const { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }, ).without([owned.path, sections.path]); expect(manifest.entries, [merged]); - expect(manifest.plugins, {'vgv-wingspan': '0.0.5'}); + expect(manifest.plugins, { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }); }); test('replaces the plugin versions, keeping the entries', () { - final manifest = InstallManifest( - const [owned], - const {'vgv-wingspan': '0.0.4'}, - ).withPlugins(const {'vgv-wingspan': '0.0.5'}); + final manifest = + InstallManifest( + const [owned], + const { + Harness.codex: {'vgv-wingspan': '0.0.4'}, + }, + ).withPlugins(const { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }); expect(manifest.entries, [owned]); - expect(manifest.plugins, {'vgv-wingspan': '0.0.5'}); + expect(manifest.plugins, { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }); }); test('equals a manifest with the same entries, in the same order', () { @@ -293,8 +327,20 @@ void main() { ), ); expect( - InstallManifest(const [owned], const {'vgv-wingspan': '0.0.5'}), - isNot(InstallManifest(const [owned], const {'vgv-wingspan': '0.0.4'})), + InstallManifest( + const [owned], + const { + Harness.codex: {'vgv-wingspan': '0.0.5'}, + }, + ), + isNot( + InstallManifest( + const [owned], + const { + Harness.codex: {'vgv-wingspan': '0.0.4'}, + }, + ), + ), ); }); From cfc0257b3564b91ecee7178aa37e2aaa2e64c612 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Thu, 22 Oct 2026 11:12:48 +0200 Subject: [PATCH 5/8] fix: keep the manifest at version 1 and the shared fix honest The format never shipped, so it stays version 1 with its plugin versions per harness: a plugins map that is not per harness is dropped and the entries are read as before. The Shared section now names the plugins its readers were installed from, and prints no fix at all when they differ, since one install cannot fix the skills they share without adding a plugin one of them was left without. --- .../src/commands/doctor/doctor_command.dart | 13 ++++++ .../commands/doctor/model/report_section.dart | 26 ++++++----- .../lib/src/model/install_manifest.dart | 9 ++-- .../commands/doctor/doctor_command_test.dart | 43 +++++++++++++++++++ .../test/src/model/install_manifest_test.dart | 8 ++-- 5 files changed, 80 insertions(+), 19 deletions(-) diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index aff9c289..ea3eb8f5 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -260,6 +260,7 @@ class DoctorCommand({ ReportSection( title: 'Shared', installs: readsShared, + plugins: _shared(manifest, readsShared), findings: _entryFindings(null, manifest, home, plan), ), for (final harness in Harness.values) @@ -307,6 +308,18 @@ class DoctorCommand({ ); } + /// The plugins every harness of [readers] was installed from, or null + /// when they differ, since one `install` cannot then fix the skills they + /// share without adding a plugin one of them was left without. + Set? _shared(InstallManifest manifest, Set readers) { + final selections = >{}; + for (final harness in readers) { + final selected = _selected(manifest.plugins[harness] ?? const {}); + selections[(selected.toList()..sort()).join(' ')] = selected; + } + return selections.length == 1 ? selections.values.single : null; + } + /// The plugin names a fix must name, or none when [recorded] holds every /// pinned plugin, which `install` writes without being asked. Set _selected(Map recorded) => diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart index 58a990cb..b27919cd 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart @@ -16,8 +16,10 @@ class const ReportSection({ /// What was found, in the order it was found. required final List findings, - /// The plugins the fix installs, or empty for every pinned one. - final Set plugins = const {}, + /// The plugins the fix installs, empty for every pinned one, or null when + /// the harnesses were installed from different ones and no one `install` + /// fixes them all. + final Set? plugins = const {}, }) { /// Whether anything was found that is wrong. bool get hasProblems => findings.any((finding) => finding.isProblem); @@ -26,12 +28,16 @@ class const ReportSection({ bool get hasNotices => findings.any((finding) => finding.isNotice); /// The one `install` command that fixes every finding `install` can fix, - /// or null when there are none. - FixCommand? get fix => findings.any((finding) => finding.needsInstall) - ? FixCommand( - harnesses: installs, - force: findings.any((finding) => finding.needsForce), - plugins: plugins, - ) - : null; + /// or null when there are none, or when no one command fixes them. + FixCommand? get fix { + if (plugins case final plugins? + when findings.any((finding) => finding.needsInstall)) { + return FixCommand( + harnesses: installs, + force: findings.any((finding) => finding.needsForce), + plugins: plugins, + ); + } + return null; + } } diff --git a/packages/vgv_ai_cli/lib/src/model/install_manifest.dart b/packages/vgv_ai_cli/lib/src/model/install_manifest.dart index 47b19695..2ad537cc 100644 --- a/packages/vgv_ai_cli/lib/src/model/install_manifest.dart +++ b/packages/vgv_ai_cli/lib/src/model/install_manifest.dart @@ -196,10 +196,9 @@ class InstallManifest extends Equatable { /// /// A malformed manifest is empty: an unrecorded file is the user's, which /// is the safe way to be wrong. For the same reason, an entry whose path - /// is absolute or leads out of [home] is dropped, and so is a manifest - /// from an older version, which recorded its plugin versions for the whole - /// home rather than per harness. One from a newer version is null, because - /// writing over it would lose everything it records. + /// is absolute or leads out of [home] is dropped, as are plugin versions + /// that are not written per harness. One from a newer version is null, + /// because writing over it would lose everything it records. static InstallManifest? decode(String source, {required String home}) { final Object? json; try { @@ -250,7 +249,7 @@ class InstallManifest extends Equatable { } /// The format [encode] writes. - static const version = 3; + static const version = 1; final Map _entries; diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index d3216494..013a1247 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -769,6 +769,49 @@ void main() { verifyInfo('✓ Gemini'); }); + test('the shared fix names the plugins its readers share', () async { + install([ + owned('.agents/skills/a/SKILL.md'), + owned('.codex/agents/a.toml', harness: Harness.codex), + ]); + store.save( + store.load()!.withPlugins({ + Harness.codex: {wingspan.name: wingspan.version}, + }), + ); + homeFile('.agents/skills/a/SKILL.md').deleteSync(); + + await runDoctor(); + + verifyInfo(' missing ~/.agents/skills/a/SKILL.md'); + verifyInfo( + ' Run vgv_ai install --only codex --plugin ${wingspan.name}', + ); + }); + + test('the shared fix is left out when its readers differ', () async { + install([ + owned('.agents/skills/a/SKILL.md'), + owned('.codex/agents/a.toml', harness: Harness.codex), + owned('.gemini/agents/a.md', harness: Harness.gemini), + ]); + store.save( + store.load()!.withPlugins({ + Harness.codex: {wingspan.name: wingspan.version}, + Harness.gemini: pinned, + }), + ); + homeFile('.agents/skills/a/SKILL.md').deleteSync(); + + await runDoctor(); + + verifyInfo(' missing ~/.agents/skills/a/SKILL.md'); + verifyNever( + () => + logger.info(' Run vgv_ai install --only gemini --only codex'), + ); + }); + test('are not reported for a harness VGV has not configured', () async { installWith({}); detect(const [Harness.codex, Harness.pi]); diff --git a/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart b/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart index 779550e8..3d4d07dd 100644 --- a/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart +++ b/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart @@ -97,7 +97,7 @@ void main() { ).encode(home: home), ) as Map; - expect(json['version'], 3); + expect(json['version'], 1); expect(json['plugins'], { 'codex': {'vgv-wingspan': '0.0.5'}, }); @@ -183,17 +183,17 @@ void main() { ); }); - test('is empty for the version before this one', () { + test('keeps the entries when the plugins are not per harness', () { final manifest = InstallManifest.decode( jsonEncode({ - 'version': InstallManifest.version - 1, + 'version': InstallManifest.version, 'entries': [onDisk(owned)], 'plugins': {'vgv-wingspan': '0.0.5'}, }), home: home, )!; - expect(manifest.entries, isEmpty); + expect(manifest.entries.map((e) => e.path), [owned.path]); expect(manifest.plugins, isEmpty); }); From fc9427b2a177f43ca517770aa3983893c8ff094c Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Thu, 22 Oct 2026 11:35:53 +0200 Subject: [PATCH 6/8] test: name the manifest decoding tests for what they check Neither the flat document nor a plugins map that is not per harness is a format this CLI ever wrote, so the tests say what decoding does with a document it cannot read instead of naming an older format. --- packages/vgv_ai_cli/test/src/model/install_manifest_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart b/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart index 3d4d07dd..cc87bc8b 100644 --- a/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart +++ b/packages/vgv_ai_cli/test/src/model/install_manifest_test.dart @@ -130,7 +130,7 @@ void main() { expect( decode(jsonEncode({owned.path: 'hash'}))!.entries, isEmpty, - reason: 'the flat format before version 1 never shipped', + reason: 'a document with no version is not a manifest', ); }); @@ -183,7 +183,7 @@ void main() { ); }); - test('keeps the entries when the plugins are not per harness', () { + test('keeps the entries when it cannot read the plugin versions', () { final manifest = InstallManifest.decode( jsonEncode({ 'version': InstallManifest.version, From deaa5e3363089a85c778121e1155138989838d93 Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Mon, 26 Oct 2026 11:13:20 +0100 Subject: [PATCH 7/8] fix: close the report only when one command fixes it A section's fix is null both when install changes nothing it found and when its harnesses were installed from different plugins, so the closing line could promise to fix everything while leaving a shared skill out of the command. A section that install can fix but that no one command covers now leaves the report without that line. --- .../src/commands/doctor/doctor_command.dart | 6 +++- .../commands/doctor/model/report_section.dart | 5 ++++ .../commands/doctor/doctor_command_test.dart | 30 +++++++++++++++++++ .../doctor/model/report_section_test.dart | 14 +++++++++ 4 files changed, 54 insertions(+), 1 deletion(-) diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index ea3eb8f5..3191ab2b 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -93,7 +93,11 @@ class DoctorCommand({ if (reports.isEmpty) _logger.info(_nothingToCheck(manifest)); - final fix = FixCommand.combine(reports.map((report) => report.fix)); + // A section install can fix but that no one command covers would make + // the closing line promise more than it does, so there is none. + final fix = reports.any((report) => report.needsItsOwnFix) + ? null + : FixCommand.combine(reports.map((report) => report.fix)); if (fix != null) { _logger ..info('') diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart index b27919cd..95741356 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/report_section.dart @@ -27,6 +27,11 @@ class const ReportSection({ /// Whether anything was found that is worth knowing, though not wrong. bool get hasNotices => findings.any((finding) => finding.isNotice); + /// Whether `install` fixes something here that [fix] cannot say in one + /// command, because the harnesses were installed from different plugins. + bool get needsItsOwnFix => + fix == null && findings.any((finding) => finding.needsInstall); + /// The one `install` command that fixes every finding `install` can fix, /// or null when there are none, or when no one command fixes them. FixCommand? get fix { diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index 013a1247..38ee1952 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -812,6 +812,36 @@ void main() { ); }); + test( + 'nothing closes the report when the shared fix is left out', + () async { + install([ + owned('.agents/skills/a/SKILL.md'), + owned('.codex/agents/a.toml', harness: Harness.codex), + owned('.gemini/agents/a.md', harness: Harness.gemini), + ]); + store.save( + store.load()!.withPlugins({ + Harness.codex: {wingspan.name: wingspan.version}, + Harness.gemini: pinned, + }), + ); + homeFile('.agents/skills/a/SKILL.md').deleteSync(); + homeFile('.codex/agents/a.toml').deleteSync(); + + await runDoctor(); + + verifyInfo( + ' Run vgv_ai install --only codex --plugin ${wingspan.name}', + ); + verifyNever( + () => logger.info( + any(that: endsWith('to fix everything install can.')), + ), + ); + }, + ); + test('are not reported for a harness VGV has not configured', () async { installWith({}); detect(const [Harness.codex, Harness.pi]); diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/model/report_section_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/model/report_section_test.dart index d3756eba..0aa9825a 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/model/report_section_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/model/report_section_test.dart @@ -75,6 +75,20 @@ void main() { ); }); + test('needs its own fix when no one command fixes what it found', () { + const unfixable = ReportSection( + title: 'Shared', + installs: {Harness.codex, Harness.gemini}, + findings: [Finding.missing('a')], + plugins: null, + ); + + expect(unfixable.fix, isNull); + expect(unfixable.needsItsOwnFix, isTrue); + expect(section(const [Finding.missing('a')]).needsItsOwnFix, isFalse); + expect(section(const []).needsItsOwnFix, isFalse); + }); + test('has no fix when install changes nothing it found', () { expect(section(const []).fix, isNull); expect( From 6f0d1dcadca25f432918ddbe5114b8036f49d9bc Mon Sep 17 00:00:00 2001 From: Dominik Simonik Date: Mon, 26 Oct 2026 11:20:48 +0100 Subject: [PATCH 8/8] refactor: compare plugin selections as sets Grouping harnesses by a joined string made the plugin names part of a key format, and splitting it back assumed none of them holds a space. An EqualityMap and an EqualitySet compare the selections directly, so nothing is joined, split, or taken as the one to compare the rest with. A harness the manifest records no plugins for now carries every pinned name rather than null, which the resolver already treats the same way. --- .../src/commands/doctor/doctor_command.dart | 44 ++++++++++++------- .../commands/doctor/model/fix_command.dart | 8 ++-- .../commands/doctor/doctor_command_test.dart | 4 +- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart index 3191ab2b..be60dcf3 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/doctor_command.dart @@ -1,6 +1,7 @@ import 'dart:io' show Platform; import 'package:args/command_runner.dart'; +import 'package:collection/collection.dart'; import 'package:file/file.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:path/path.dart' as p; @@ -175,11 +176,10 @@ class DoctorCommand({ final progress = _logger.progress('Resolving plugins'); final changes = []; - for (final MapEntry(key: names, value: harnesses) in _byPlugins( + for (final MapEntry(key: only, value: harnesses) in _byPlugins( manifest, selected, ).entries) { - final only = names.isEmpty ? null : names.split(' ').toSet(); switch (await _resolver.resolve(home, only: only)) { case ResolvedAssets(:final assets): final translation = await _translator.translate( @@ -203,21 +203,35 @@ class DoctorCommand({ return ContentPlan(changes); } - /// [selected] grouped by the plugins each harness was installed from, - /// keyed by their names in order, and by the empty string for a harness - /// the manifest records none for, which is checked against every pin. - Map> _byPlugins( + /// [selected] grouped by the plugin names each harness was installed + /// from, which the map compares as sets rather than by identity. + Map, Set> _byPlugins( InstallManifest manifest, Set selected, ) { - final groups = >{}; + final groups = EqualityMap, Set>(_sets); for (final harness in selected) { - final recorded = manifest.plugins[harness]?.keys.toList() ?? []; - groups.putIfAbsent((recorded..sort()).join(' '), () => {}).add(harness); + groups + .putIfAbsent(_installedFrom(manifest, harness), () => {}) + .add(harness); } return groups; } + /// How a set of plugin names is compared, since two of them are the same + /// selection whatever order they were built in. + static const Equality> _sets = SetEquality(); + + /// The plugins [harness] was installed from, which for one the manifest + /// records none for is every pinned plugin, as `install` writes without + /// `--plugin`. + Set _installedFrom(InstallManifest manifest, Harness harness) { + final recorded = manifest.plugins[harness]?.keys.toSet() ?? const {}; + return recorded.isEmpty + ? {for (final plugin in vgvPlugins) plugin.name} + : recorded; + } + /// An unchecked plan, having failed [progress] and listed [reasons] for /// why the CLI could not do [problem], such as `resolve `. ContentPlan _unchecked( @@ -316,12 +330,12 @@ class DoctorCommand({ /// when they differ, since one `install` cannot then fix the skills they /// share without adding a plugin one of them was left without. Set? _shared(InstallManifest manifest, Set readers) { - final selections = >{}; - for (final harness in readers) { - final selected = _selected(manifest.plugins[harness] ?? const {}); - selections[(selected.toList()..sort()).join(' ')] = selected; - } - return selections.length == 1 ? selections.values.single : null; + final selections = EqualitySet>(_sets) + ..addAll([ + for (final harness in readers) + _selected(manifest.plugins[harness] ?? const {}), + ]); + return selections.length == 1 ? selections.single : null; } /// The plugin names a fix must name, or none when [recorded] holds every diff --git a/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart b/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart index b2d736e7..c6bd7753 100644 --- a/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart +++ b/packages/vgv_ai_cli/lib/src/commands/doctor/model/fix_command.dart @@ -25,14 +25,14 @@ class const FixCommand({ final all = fixes.nonNulls.toList(); if (all.isEmpty) return null; - final plugins = all.first.plugins; - const equality = SetEquality(); - if (all.any((fix) => !equality.equals(fix.plugins, plugins))) return null; + final plugins = EqualitySet>(const SetEquality()) + ..addAll(all.map((fix) => fix.plugins)); + if (plugins.length > 1) return null; return FixCommand( harnesses: {for (final fix in all) ...fix.harnesses}, force: all.any((fix) => fix.force), - plugins: plugins, + plugins: plugins.single, ); } diff --git a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart index 38ee1952..d7995d3f 100644 --- a/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart +++ b/packages/vgv_ai_cli/test/src/commands/doctor/doctor_command_test.dart @@ -687,7 +687,9 @@ void main() { expect(exitCode, ExitCode.success.code); verifyInfo(' ${wingspan.name} ${wingspan.version} not installed'); // Nothing was recorded, so every pinned plugin is still checked. - expect(resolver.resolved, [null]); + expect(resolver.resolved, [ + {for (final plugin in vgvPlugins) plugin.name}, + ]); }); test(