From c2dbf3c12c7e1233484fff601357e58b67daecbb Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:42:12 +0800 Subject: [PATCH 01/19] fix(extensions): preserve Claude hooks in dual manifests --- .../8539-dual-manifest-extension-hooks.md | 18 + docs/users/features/hooks.md | 6 + .../extensions/tabs/DiscoverTab.test.tsx | 4 + .../extensions/tabs/DiscoverTab.tsx | 4 +- packages/core/src/config/config.ts | 2 + .../src/extension/claude-converter.test.ts | 51 ++ .../core/src/extension/claude-converter.ts | 124 +++-- .../src/extension/extension-converter.test.ts | 468 ++++++++++++++++++ .../core/src/extension/extension-converter.ts | 150 +++++- .../src/extension/extensionManager.test.ts | 31 ++ .../core/src/extension/extensionManager.ts | 3 + packages/core/src/extension/github.ts | 2 + .../core/src/extension/marketplace.test.ts | 12 + packages/core/src/extension/marketplace.ts | 6 + .../core/src/extension/sourceRegistry.test.ts | 32 ++ packages/core/src/extension/sourceRegistry.ts | 86 +++- 16 files changed, 909 insertions(+), 90 deletions(-) create mode 100644 .qwen/e2e-tests/8539-dual-manifest-extension-hooks.md create mode 100644 packages/core/src/extension/extension-converter.test.ts diff --git a/.qwen/e2e-tests/8539-dual-manifest-extension-hooks.md b/.qwen/e2e-tests/8539-dual-manifest-extension-hooks.md new file mode 100644 index 00000000000..1c0d97da57e --- /dev/null +++ b/.qwen/e2e-tests/8539-dual-manifest-extension-hooks.md @@ -0,0 +1,18 @@ +# Dual-manifest extension hooks reviewer plan + +## Scope + +Verify a trusted test extension containing `gemini-extension.json`, `.claude-plugin/plugin.json`, a Claude hooks file, `AGENTS.md`, and a Gemini TOML slash command. Do not run third-party Ponytail hook code for this review. + +## How to verify + +1. Build Qwen Code from this branch. +2. Create a local fixture extension whose Gemini manifest declares `contextFileName: "AGENTS.md"`, whose Claude manifest points to a hooks JSON file, and whose `commands/` directory contains a valid TOML command. +3. Make the fixture's `SessionStart` hook write a fixed marker to a temporary test directory. Install the fixture once as a standalone extension and once through a local Claude marketplace entry. +4. Remove or move the original fixture source after installation, then start a fresh session for each installed form. +5. Confirm the marker is written by the hook from the installed extension path, the `AGENTS.md` instruction is present in the session context, and the converted slash command is listed and can be invoked. +6. Uninstall both fixture extensions and remove the temporary marker directory. + +## Expected result + +Both install forms retain the Gemini context and TOML command while registering the Claude hook. The hook command resolves `${CLAUDE_PLUGIN_ROOT}` to the final installed extension directory and does not depend on the original source or conversion directory. diff --git a/docs/users/features/hooks.md b/docs/users/features/hooks.md index c2010e0e059..45dde0f0b8c 100644 --- a/docs/users/features/hooks.md +++ b/docs/users/features/hooks.md @@ -28,6 +28,12 @@ Hooks are user-defined scripts or programs that are automatically executed by Qw - Integrate with external systems and services - Modify tool inputs or responses programmatically +### Hooks provided by extensions + +Extensions can provide hooks inline in `qwen-extension.json` or reference a hooks JSON file from the manifest. Claude-compatible extensions can likewise declare an inline `hooks` object or a relative hooks file in `.claude-plugin/plugin.json`. + +For extensions that contain both `gemini-extension.json` and a root `.claude-plugin/plugin.json`, Qwen Code keeps the Gemini extension resources, context file, and settings while also importing the Claude-compatible hooks for that root. For a named marketplace install, this merge happens only when the selected marketplace entry points to the repository root; fields on that entry still override `plugin.json` as usual. `${CLAUDE_PLUGIN_ROOT}` in imported hooks resolves to the installed extension directory. Install third-party extensions only from sources you trust because command hooks execute with your user permissions. + ## Hook Types Qwen Code supports four hook executor types: diff --git a/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.test.tsx b/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.test.tsx index 53afe8a30a6..5c4a28b069d 100644 --- a/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.test.tsx +++ b/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.test.tsx @@ -59,6 +59,7 @@ describe('DiscoverTab', () => { name: 'demo', marketplaceName: 'market', installSource: 'owner/demo', + pluginSourceKind: 'extension-root', installed: false, } as DiscoveredPlugin; const manager = { @@ -94,6 +95,9 @@ describe('DiscoverTab', () => { }); await waitFor(() => expect(manager.installExtension).toHaveBeenCalled()); + expect(mockParseInstallSource).toHaveBeenCalledWith('owner/demo', { + pluginSourceKind: 'extension-root', + }); expect(manager.installExtension).toHaveBeenCalledWith( { type: 'git', source: 'owner/demo' }, undefined, diff --git a/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.tsx b/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.tsx index 546588d8cb0..7e13e2eb46b 100644 --- a/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.tsx +++ b/packages/cli/src/ui/components/extensions/tabs/DiscoverTab.tsx @@ -219,7 +219,9 @@ export const DiscoverTab = ({ for (const plugin of targets) { let ext; try { - const metadata = await parseInstallSource(plugin.installSource); + const metadata = await parseInstallSource(plugin.installSource, { + pluginSourceKind: plugin.pluginSourceKind, + }); ext = await extensionManager.installExtension( metadata, undefined, diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index ad186d82428..d5be148d1fb 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -674,6 +674,7 @@ function normalizeGitCoAuthor(value: GitCoAuthorParam | undefined): { export type ExtensionOriginSource = 'QwenCode' | 'Claude' | 'Gemini'; export type ExtensionNetworkPolicy = 'public'; +export type ExtensionPluginSourceKind = 'marketplace-entry' | 'extension-root'; export interface ExtensionInstallMetadata { source: string; @@ -686,6 +687,7 @@ export interface ExtensionInstallMetadata { allowPreRelease?: boolean; marketplaceConfig?: ClaudeMarketplaceConfig; pluginName?: string; + pluginSourceKind?: ExtensionPluginSourceKind; networkPolicy?: ExtensionNetworkPolicy; } diff --git a/packages/core/src/extension/claude-converter.test.ts b/packages/core/src/extension/claude-converter.test.ts index 04f3494fa03..91f4effff22 100644 --- a/packages/core/src/extension/claude-converter.test.ts +++ b/packages/core/src/extension/claude-converter.test.ts @@ -23,6 +23,7 @@ import { import { cloneFromGit, downloadFromGitHubRelease } from './github.js'; import { HookType } from '../hooks/types.js'; import { performVariableReplacement } from './variables.js'; +import { ExtensionStorage } from './storage.js'; // The git-subdir source clones a repo; stub the network clone so the security // guards around the cloned subdirectory can be exercised against a real fs. @@ -228,6 +229,56 @@ describe('convertClaudePluginPackage', () => { } }); + it('cleans the temporary plugin staging directory after conversion', async () => { + const pluginSourceDir = path.join(testDir, 'root-plugin'); + const marketplaceDir = path.join(pluginSourceDir, '.claude-plugin'); + fs.mkdirSync(marketplaceDir, { recursive: true }); + fs.writeFileSync( + path.join(marketplaceDir, 'marketplace.json'), + JSON.stringify({ + name: 'test-marketplace', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'root-plugin', + version: '1.0.0', + source: './', + }, + ], + }), + 'utf-8', + ); + + const createTmpDir = ExtensionStorage.createTmpDir.bind(ExtensionStorage); + const tempDirs: string[] = []; + const createTmpDirSpy = vi + .spyOn(ExtensionStorage, 'createTmpDir') + .mockImplementation(async () => { + const tempDir = await createTmpDir(); + tempDirs.push(tempDir); + return tempDir; + }); + let outputDir: string | undefined; + + try { + const result = await convertClaudePluginPackage( + pluginSourceDir, + 'root-plugin', + ); + outputDir = result.convertedDir; + + expect(tempDirs).toHaveLength(2); + expect(outputDir).toBe(tempDirs[1]); + expect(fs.existsSync(tempDirs[0])).toBe(false); + expect(fs.existsSync(outputDir)).toBe(true); + } finally { + createTmpDirSpy.mockRestore(); + for (const tempDir of tempDirs) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + } + }); + it('should only collect specified skills when config provides explicit list', async () => { // Setup: Create a plugin source with multiple skills const pluginSourceDir = path.join(testDir, 'plugin-source'); diff --git a/packages/core/src/extension/claude-converter.ts b/packages/core/src/extension/claude-converter.ts index 0c47a7c04a4..1f2c6c6cba8 100644 --- a/packages/core/src/extension/claude-converter.ts +++ b/packages/core/src/extension/claude-converter.ts @@ -18,7 +18,6 @@ import type { } from '../config/config.js'; import type { HookEventName, HookDefinition } from '../hooks/types.js'; import { cloneFromGit, downloadFromGitHubRelease } from './github.js'; -import { createHash } from 'node:crypto'; import { copyDirectory, isPathWithin, @@ -453,6 +452,7 @@ export async function convertClaudePluginPackage( pluginName: string, networkPolicy?: ExtensionInstallMetadata['networkPolicy'], signal?: AbortSignal, + preserveHookVariables = false, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { signal?.throwIfAborted(); // Step 1: Load marketplace.json @@ -487,64 +487,70 @@ export async function convertClaudePluginPackage( } // Step 2: Resolve plugin source directory based on source field - const pluginDir = path.join( - extensionDir, - `plugin${createHash('sha256').update(`${extensionDir}/${pluginName}`).digest('hex')}`, - ); - await fs.promises.mkdir(pluginDir, { recursive: true }); - - const pluginSource = await resolvePluginSource( - marketplacePlugin, - extensionDir, - pluginDir, - networkPolicy, - signal, - ); + const pluginDir = await ExtensionStorage.createTmpDir(); + try { + const pluginSource = await resolvePluginSource( + marketplacePlugin, + extensionDir, + pluginDir, + networkPolicy, + signal, + ); - if (!fs.existsSync(pluginSource)) { - throw new Error(`Plugin source directory not found: ${pluginSource}`); - } + if (!fs.existsSync(pluginSource)) { + throw new Error(`Plugin source directory not found: ${pluginSource}`); + } - // Step 3: Load and merge plugin.json if exists (based on strict mode) - const strict = marketplacePlugin.strict ?? false; - let mergedConfig: ClaudePluginConfig; + // Step 3: Load and merge plugin.json if exists (based on strict mode) + const strict = marketplacePlugin.strict ?? false; + let mergedConfig: ClaudePluginConfig; - const pluginJsonPath = path.join( - pluginSource, - '.claude-plugin', - 'plugin.json', - ); - if (strict && !fs.existsSync(pluginJsonPath)) { - throw new Error(`Strict mode requires plugin.json at ${pluginJsonPath}`); - } - // Treat a symlinked plugin.json (pointing outside the source) as absent - // rather than reading an arbitrary host file into the merged config. - const pluginJsonSafe = - fs.existsSync(pluginJsonPath) && - realPathWithin(pluginJsonPath, pluginSource); - if (pluginJsonSafe) { - const pluginContent = fs.readFileSync(pluginJsonPath, 'utf-8'); - const pluginConfig: ClaudePluginConfig = JSON.parse(pluginContent); - mergedConfig = mergeClaudeConfigs(marketplacePlugin, pluginConfig); - } else { - // `existsSync` follows symlinks, so the strict check at line 500 passes - // when plugin.json is a symlink to an existing host file — but the file is - // not trusted (`realPathWithin` rejected it). Strict mode must fail here - // rather than silently fall back to the marketplace entry. - if (strict) { - throw new Error( - `Strict mode requires a trusted plugin.json at ${pluginJsonPath}`, - ); + const pluginJsonPath = path.join( + pluginSource, + '.claude-plugin', + 'plugin.json', + ); + if (strict && !fs.existsSync(pluginJsonPath)) { + throw new Error(`Strict mode requires plugin.json at ${pluginJsonPath}`); } - if (fs.existsSync(pluginJsonPath)) { - debugLogger.warn( - `Ignoring plugin.json at ${pluginJsonPath}; it resolves through a symlink outside the plugin.`, - ); + // Treat a symlinked plugin.json (pointing outside the source) as absent + // rather than reading an arbitrary host file into the merged config. + const pluginJsonSafe = + fs.existsSync(pluginJsonPath) && + realPathWithin(pluginJsonPath, pluginSource); + if (pluginJsonSafe) { + const pluginContent = fs.readFileSync(pluginJsonPath, 'utf-8'); + const pluginConfig: ClaudePluginConfig = JSON.parse(pluginContent); + mergedConfig = mergeClaudeConfigs(marketplacePlugin, pluginConfig); + } else { + // `existsSync` follows symlinks, so the strict check above passes when + // plugin.json points to an existing host file. Strict mode must fail + // here rather than silently fall back to the marketplace entry. + if (strict) { + throw new Error( + `Strict mode requires a trusted plugin.json at ${pluginJsonPath}`, + ); + } + if (fs.existsSync(pluginJsonPath)) { + debugLogger.warn( + `Ignoring plugin.json at ${pluginJsonPath}; it resolves through a symlink outside the plugin.`, + ); + } + mergedConfig = marketplacePlugin as ClaudePluginConfig; } - mergedConfig = marketplacePlugin as ClaudePluginConfig; - } - return buildQwenExtensionFromPlugin(pluginSource, mergedConfig); + return await buildQwenExtensionFromPlugin( + pluginSource, + mergedConfig, + preserveHookVariables, + ); + } finally { + try { + await fs.promises.rm(pluginDir, { recursive: true, force: true }); + } catch { + // Best-effort cleanup must not mask conversion errors or a valid result. + } + } } /** @@ -594,6 +600,7 @@ function resolvePluginRelativeFile( async function buildQwenExtensionFromPlugin( pluginSource: string, mergedConfig: ClaudePluginConfig, + preserveHookVariables = false, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { // Resolve MCP servers from a JSON file path if needed. if (mergedConfig.mcpServers && typeof mergedConfig.mcpServers === 'string') { @@ -677,7 +684,9 @@ async function buildQwenExtensionFromPlugin( }; } - mergedConfig.hooks = substituteHookVariables(hooksData, pluginSource); + mergedConfig.hooks = preserveHookVariables + ? hooksData + : substituteHookVariables(hooksData, pluginSource); } catch (error) { debugLogger.warn( `Failed to parse hooks file ${hooksPath}: ${error instanceof Error ? error.message : String(error)}`, @@ -722,6 +731,7 @@ async function buildQwenExtensionFromPlugin( */ export async function convertClaudePluginStandalone( extensionDir: string, + preserveHookVariables = false, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { const pluginJsonPath = path.join( extensionDir, @@ -794,7 +804,11 @@ export async function convertClaudePluginStandalone( } } - return buildQwenExtensionFromPlugin(extensionDir, mergedConfig); + return buildQwenExtensionFromPlugin( + extensionDir, + mergedConfig, + preserveHookVariables, + ); } /** diff --git a/packages/core/src/extension/extension-converter.test.ts b/packages/core/src/extension/extension-converter.test.ts new file mode 100644 index 00000000000..030742db120 --- /dev/null +++ b/packages/core/src/extension/extension-converter.test.ts @@ -0,0 +1,468 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { convertGeminiOrClaudeExtension } from './extension-converter.js'; +import { ExtensionManager, type ExtensionConfig } from './extensionManager.js'; +import { ExtensionStore } from './extension-store.js'; + +describe('convertGeminiOrClaudeExtension', () => { + let extensionDir: string; + let convertedDir: string | undefined; + let installRoot: string; + let installedDir: string | undefined; + + beforeEach(() => { + extensionDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dual-extension-')); + installRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'dual-installed-')); + }); + + afterEach(() => { + if (convertedDir && fs.existsSync(convertedDir)) { + fs.rmSync(convertedDir, { recursive: true, force: true }); + } + fs.rmSync(installRoot, { recursive: true, force: true }); + fs.rmSync(extensionDir, { recursive: true, force: true }); + }); + + it.each([ + { + installKind: 'standalone', + pluginName: undefined, + marketplacePluginName: 'ponytail', + marketplaceSource: './', + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: undefined, + includeMarketplace: true, + }, + { + installKind: 'named root', + pluginName: 'ponytail', + marketplacePluginName: 'ponytail', + marketplaceSource: './', + marketplaceHooks: './hooks/marketplace-hooks.json', + expectedHookPath: 'scripts/marketplace-session-start.sh', + pluginSourceKind: undefined, + includeMarketplace: true, + }, + { + installKind: 'aliased root', + pluginName: 'ponytail-alias', + marketplacePluginName: 'ponytail-alias', + marketplaceSource: './', + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: undefined, + includeMarketplace: true, + }, + { + installKind: 'direct root alias', + pluginName: 'ponytail-alias', + marketplacePluginName: 'different-plugin', + marketplaceSource: './plugins/different-plugin', + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: 'extension-root' as const, + includeMarketplace: true, + }, + { + installKind: 'legacy direct root alias', + pluginName: 'ponytail-alias', + marketplacePluginName: 'different-plugin', + marketplaceSource: './plugins/different-plugin', + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: undefined, + includeMarketplace: true, + }, + ])( + 'keeps Gemini resources and imports only selected root Claude hooks for a dual-manifest $installKind extension', + async ({ + pluginName, + marketplacePluginName, + marketplaceSource, + marketplaceHooks, + expectedHookPath, + pluginSourceKind, + includeMarketplace, + }) => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ + name: 'ponytail', + version: '4.8.4', + contextFileName: 'AGENTS.md', + settings: [ + { + name: 'Ponytail mode', + envVar: 'PONYTAIL_MODE', + description: 'Select the ponytail mode', + }, + ], + }), + 'utf-8', + ); + fs.writeFileSync(path.join(extensionDir, 'AGENTS.md'), '# Ponytail'); + + const claudePluginDir = path.join(extensionDir, '.claude-plugin'); + fs.mkdirSync(claudePluginDir, { recursive: true }); + if (includeMarketplace) { + fs.writeFileSync( + path.join(claudePluginDir, 'marketplace.json'), + JSON.stringify({ + name: 'ponytail', + owner: { name: 'Ponytail' }, + plugins: [ + { + name: marketplacePluginName, + source: marketplaceSource, + hooks: marketplaceHooks, + }, + ], + }), + 'utf-8', + ); + } + fs.writeFileSync( + path.join(claudePluginDir, 'plugin.json'), + JSON.stringify({ + name: 'ponytail', + version: '4.8.4', + description: 'A dual-manifest extension', + hooks: './hooks/claude-codex-hooks.json', + }), + 'utf-8', + ); + + const hooksDir = path.join(extensionDir, 'hooks'); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.writeFileSync( + path.join(hooksDir, 'claude-codex-hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: '${CLAUDE_PLUGIN_ROOT}/scripts/session-start.sh', + }, + ], + }, + ], + }, + }), + 'utf-8', + ); + fs.writeFileSync( + path.join(hooksDir, 'marketplace-hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: + '${CLAUDE_PLUGIN_ROOT}/scripts/marketplace-session-start.sh', + }, + ], + }, + ], + }, + }), + 'utf-8', + ); + + const commandsDir = path.join(extensionDir, 'commands'); + fs.mkdirSync(commandsDir, { recursive: true }); + fs.writeFileSync( + path.join(commandsDir, 'ponytail.toml'), + 'description = "Use ponytail mode"\nprompt = "Keep it simple"\n', + 'utf-8', + ); + + const sourceEntriesBefore = fs.readdirSync(extensionDir).sort(); + const converted = await convertGeminiOrClaudeExtension( + extensionDir, + pluginName, + undefined, + undefined, + pluginSourceKind, + ); + expect(fs.readdirSync(extensionDir).sort()).toEqual(sourceEntriesBefore); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(converted.originSource).toBe('Gemini'); + expect(config.name).toBe('ponytail'); + expect(config.contextFileName).toBe('AGENTS.md'); + expect(config.settings).toEqual([ + { + name: 'Ponytail mode', + envVar: 'PONYTAIL_MODE', + description: 'Select the ponytail mode', + }, + ]); + expect( + ( + config.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe( + expectedHookPath + ? '${CLAUDE_PLUGIN_ROOT}/' + expectedHookPath + : undefined, + ); + expect( + fs.existsSync( + path.join(converted.extensionDir, 'commands', 'ponytail.md'), + ), + ).toBe(true); + expect( + fs.existsSync( + path.join(converted.extensionDir, 'commands', 'ponytail.toml'), + ), + ).toBe(false); + + installedDir = path.join(installRoot, 'ponytail'); + fs.renameSync(converted.extensionDir, installedDir); + convertedDir = undefined; + fs.rmSync(extensionDir, { recursive: true, force: true }); + + const manager = new ExtensionManager({ + workspaceDir: os.tmpdir(), + isWorkspaceTrusted: true, + extensionStore: new ExtensionStore({ + extensionsDir: installRoot, + storeDir: path.join(installRoot, '.store'), + }), + }); + const installedConfig = manager.loadExtensionConfig({ + extensionDir: installedDir, + }); + + expect(fs.existsSync(extensionDir)).toBe(false); + expect( + ( + installedConfig.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe( + expectedHookPath + ? path.join(installedDir, expectedHookPath) + : undefined, + ); + }, + ); + + it('treats pluginName as an alias for an explicit Claude extension-root install', async () => { + const claudeDir = path.join(extensionDir, '.claude-plugin'); + const hooksDir = path.join(extensionDir, 'hooks'); + fs.mkdirSync(claudeDir, { recursive: true }); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.writeFileSync( + path.join(claudeDir, 'marketplace.json'), + JSON.stringify({ + name: 'unrelated-marketplace', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'different-plugin', + source: './plugins/different-plugin', + }, + ], + }), + 'utf-8', + ); + fs.writeFileSync( + path.join(claudeDir, 'plugin.json'), + JSON.stringify({ + name: 'root-plugin', + version: '1.0.0', + hooks: './hooks/hooks.json', + }), + 'utf-8', + ); + fs.writeFileSync( + path.join(hooksDir, 'hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: '${CLAUDE_PLUGIN_ROOT}/scripts/start.sh', + }, + ], + }, + ], + }, + }), + 'utf-8', + ); + + const converted = await convertGeminiOrClaudeExtension( + extensionDir, + 'root-alias', + undefined, + undefined, + 'extension-root', + ); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(converted.originSource).toBe('Claude'); + expect(config.name).toBe('root-plugin'); + expect( + ( + config.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe('${CLAUDE_PLUGIN_ROOT}/scripts/start.sh'); + }); + + it('installs a selected same-named subdirectory instead of the marketplace root Gemini extension', async () => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ + name: 'marketplace-root', + version: '1.0.0', + contextFileName: 'ROOT.md', + }), + 'utf-8', + ); + fs.writeFileSync(path.join(extensionDir, 'ROOT.md'), '# Root'); + + const rootClaudeDir = path.join(extensionDir, '.claude-plugin'); + fs.mkdirSync(rootClaudeDir, { recursive: true }); + fs.writeFileSync( + path.join(rootClaudeDir, 'marketplace.json'), + JSON.stringify({ + name: 'marketplace-root', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'ponytail', + source: './plugins/ponytail', + }, + ], + }), + 'utf-8', + ); + + const pluginDir = path.join(extensionDir, 'plugins', 'ponytail'); + const pluginManifestDir = path.join(pluginDir, '.claude-plugin'); + const hooksDir = path.join(pluginDir, 'hooks'); + const scriptsDir = path.join(pluginDir, 'scripts'); + fs.mkdirSync(pluginManifestDir, { recursive: true }); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.mkdirSync(scriptsDir, { recursive: true }); + fs.writeFileSync( + path.join(pluginManifestDir, 'plugin.json'), + JSON.stringify({ + name: 'ponytail-child', + version: '2.0.0', + description: 'Selected child plugin', + hooks: './hooks/hooks.json', + }), + 'utf-8', + ); + fs.writeFileSync( + path.join(hooksDir, 'hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: '${CLAUDE_PLUGIN_ROOT}/scripts/start.sh', + }, + ], + }, + ], + }, + }), + 'utf-8', + ); + fs.writeFileSync(path.join(scriptsDir, 'start.sh'), '#!/bin/sh\n'); + + const sourceEntriesBefore = fs.readdirSync(extensionDir).sort(); + const converted = await convertGeminiOrClaudeExtension( + extensionDir, + 'ponytail', + undefined, + undefined, + 'marketplace-entry', + ); + convertedDir = converted.extensionDir; + expect(fs.readdirSync(extensionDir).sort()).toEqual(sourceEntriesBefore); + + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + expect(converted.originSource).toBe('Claude'); + expect(config.name).toBe('ponytail'); + expect(config.description).toBe('Selected child plugin'); + expect(config.contextFileName).toBeUndefined(); + expect( + ( + config.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe('${CLAUDE_PLUGIN_ROOT}/scripts/start.sh'); + expect( + fs.existsSync(path.join(converted.extensionDir, 'scripts', 'start.sh')), + ).toBe(true); + + installedDir = path.join(installRoot, 'ponytail-child'); + fs.renameSync(converted.extensionDir, installedDir); + convertedDir = undefined; + fs.rmSync(extensionDir, { recursive: true, force: true }); + + const manager = new ExtensionManager({ + workspaceDir: os.tmpdir(), + isWorkspaceTrusted: true, + extensionStore: new ExtensionStore({ + extensionsDir: installRoot, + storeDir: path.join(installRoot, '.store'), + }), + }); + const installedConfig = manager.loadExtensionConfig({ + extensionDir: installedDir, + }); + expect( + ( + installedConfig.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe(path.join(installedDir, 'scripts', 'start.sh')); + }); +}); diff --git a/packages/core/src/extension/extension-converter.ts b/packages/core/src/extension/extension-converter.ts index d3fa57a9abd..42a19e0098f 100644 --- a/packages/core/src/extension/extension-converter.ts +++ b/packages/core/src/extension/extension-converter.ts @@ -10,6 +10,7 @@ import { EXTENSIONS_CONFIG_FILENAME } from './variables.js'; import { convertGeminiExtensionPackage, isGeminiExtensionConfig, + realPathWithin, } from './gemini-converter.js'; import { convertClaudePluginPackage, @@ -18,6 +19,7 @@ import { import type { ExtensionNetworkPolicy, ExtensionOriginSource, + ExtensionPluginSourceKind, } from '../config/config.js'; export const SUPPORTED_EXTENSION_MANIFESTS = [ @@ -27,11 +29,70 @@ export const SUPPORTED_EXTENSION_MANIFESTS = [ '.claude-plugin/plugin.json', ] as const; +function removeConvertedDirectory(directory: string): void { + try { + fs.rmSync(directory, { recursive: true, force: true }); + } catch { + // Ignore cleanup errors so they do not mask the conversion result. + } +} + +type MarketplacePluginLocation = 'root' | 'other' | 'missing-marketplace'; + +function selectedMarketplacePluginLocation( + extensionDir: string, + pluginName: string, +): MarketplacePluginLocation { + const marketplacePath = path.join( + extensionDir, + SUPPORTED_EXTENSION_MANIFESTS[2], + ); + try { + fs.lstatSync(marketplacePath); + } catch { + return 'missing-marketplace'; + } + if ( + !fs.existsSync(marketplacePath) || + !realPathWithin(marketplacePath, extensionDir) + ) { + return 'other'; + } + + try { + const marketplace: unknown = JSON.parse( + fs.readFileSync(marketplacePath, 'utf-8'), + ); + if ( + typeof marketplace !== 'object' || + marketplace === null || + !Array.isArray((marketplace as { plugins?: unknown }).plugins) + ) { + return 'other'; + } + + const selectedPlugin = ( + marketplace as { plugins: Array> } + ).plugins.find((plugin) => plugin['name'] === pluginName); + if (typeof selectedPlugin?.['source'] !== 'string') { + return 'other'; + } + + return path.resolve(path.join(extensionDir, selectedPlugin['source'])) === + path.resolve(extensionDir) + ? 'root' + : 'other'; + } catch { + return 'other'; + } +} + export async function convertGeminiOrClaudeExtension( extensionDir: string, pluginName?: string, networkPolicy?: ExtensionNetworkPolicy, signal?: AbortSignal, + pluginSourceKind?: ExtensionPluginSourceKind, ): Promise<{ extensionDir: string; originSource: ExtensionOriginSource }> { signal?.throwIfAborted(); let newExtensionDir = extensionDir; @@ -40,26 +101,101 @@ export async function convertGeminiOrClaudeExtension( extensionDir, SUPPORTED_EXTENSION_MANIFESTS[0], ); - if (fs.existsSync(configFilePath)) { + const hasQwenConfig = fs.existsSync(configFilePath); + const isGeminiExtension = + !hasQwenConfig && isGeminiExtensionConfig(extensionDir); + const hasClaudePlugin = fs.existsSync( + path.join(extensionDir, SUPPORTED_EXTENSION_MANIFESTS[3]), + ); + // `pluginName` has two meanings: a selector inside a marketplace repo, or an + // alias retained for a direct plugin-root install. New metadata disambiguates + // them. Legacy metadata keeps the old manifest-first behavior so an update + // cannot suddenly replace a previously installed root Gemini/Qwen extension + // with a marketplace subplugin. + const marketplaceLocation = pluginName + ? selectedMarketplacePluginLocation(extensionDir, pluginName) + : 'missing-marketplace'; + const isExplicitMarketplaceEntry = pluginSourceKind === 'marketplace-entry'; + const isExplicitExtensionRoot = pluginSourceKind === 'extension-root'; + const selectedMarketplaceEntryUsesRoot = marketplaceLocation === 'root'; + const rootMarketplacePluginName = + pluginName && !isExplicitExtensionRoot && selectedMarketplaceEntryUsesRoot + ? pluginName + : undefined; + + // A selected subdirectory/remote marketplace plugin must win over manifests + // at the marketplace repository root. Only explicit new metadata opts into + // this precedence; legacy installs retain their previous root selection. + if ( + isExplicitMarketplaceEntry && + pluginName && + !selectedMarketplaceEntryUsesRoot + ) { + newExtensionDir = ( + await convertClaudePluginPackage( + extensionDir, + pluginName, + networkPolicy, + signal, + true, + ) + ).convertedDir; + originSource = 'Claude'; + } else if (hasQwenConfig) { newExtensionDir = extensionDir; - } else if (isGeminiExtensionConfig(extensionDir)) { + } else if (isGeminiExtension && hasClaudePlugin) { + const geminiConversion = await convertGeminiExtensionPackage(extensionDir); + let claudeConversion: + | Awaited> + | undefined; + try { + signal?.throwIfAborted(); + claudeConversion = rootMarketplacePluginName + ? await convertClaudePluginPackage( + extensionDir, + rootMarketplacePluginName, + networkPolicy, + signal, + true, + ) + : await convertClaudePluginStandalone(extensionDir, true); + const mergedConfig = { + ...geminiConversion.config, + hooks: claudeConversion.config.hooks ?? geminiConversion.config.hooks, + }; + signal?.throwIfAborted(); + fs.writeFileSync( + path.join(geminiConversion.convertedDir, EXTENSIONS_CONFIG_FILENAME), + JSON.stringify(mergedConfig, null, 2), + 'utf-8', + ); + newExtensionDir = geminiConversion.convertedDir; + originSource = 'Gemini'; + } catch (error) { + removeConvertedDirectory(geminiConversion.convertedDir); + throw error; + } finally { + if (claudeConversion) { + removeConvertedDirectory(claudeConversion.convertedDir); + } + } + } else if (isGeminiExtension) { newExtensionDir = (await convertGeminiExtensionPackage(extensionDir)) .convertedDir; originSource = 'Gemini'; - } else if (pluginName) { + } else if (pluginName && !isExplicitExtensionRoot) { newExtensionDir = ( await convertClaudePluginPackage( extensionDir, pluginName, networkPolicy, signal, + true, ) ).convertedDir; originSource = 'Claude'; - } else if ( - fs.existsSync(path.join(extensionDir, SUPPORTED_EXTENSION_MANIFESTS[3])) - ) { - newExtensionDir = (await convertClaudePluginStandalone(extensionDir)) + } else if (hasClaudePlugin) { + newExtensionDir = (await convertClaudePluginStandalone(extensionDir, true)) .convertedDir; originSource = 'Claude'; } diff --git a/packages/core/src/extension/extensionManager.test.ts b/packages/core/src/extension/extensionManager.test.ts index 9e64ffe0005..3fc02f11aa9 100644 --- a/packages/core/src/extension/extensionManager.test.ts +++ b/packages/core/src/extension/extensionManager.test.ts @@ -214,6 +214,37 @@ describe('extension tests', () => { expect(fs.existsSync(installed.path)).toBe(false); }); + it('does not re-prompt for a marketplace plugin when the source is an extension root', async () => { + const sourceDir = path.join(tempWorkspaceDir, 'direct-root-source'); + writeExtractedExtension(sourceDir, 'direct-root-extension'); + const requestChoicePlugin = vi.fn(async () => 'wrong-subplugin'); + const manager = createExtensionManager({ requestChoicePlugin }); + + const installed = await manager.installExtension( + { + type: 'local', + source: sourceDir, + originSource: 'Claude', + pluginSourceKind: 'extension-root', + marketplaceConfig: { + name: 'unrelated-marketplace', + owner: { name: 'Owner', email: 'owner@example.com' }, + plugins: [ + { + name: 'wrong-subplugin', + version: '1.0.0', + source: './plugins/wrong-subplugin', + }, + ], + }, + }, + async () => {}, + ); + + expect(requestChoicePlugin).not.toHaveBeenCalled(); + expect(installed.name).toBe('direct-root-extension'); + }); + it('commits workspace initial activation with the installed artifact', async () => { const archivePath = path.join(tempWorkspaceDir, 'workspace-ext.zip'); fs.writeFileSync(archivePath, 'archive'); diff --git a/packages/core/src/extension/extensionManager.ts b/packages/core/src/extension/extensionManager.ts index 31f308b5ab6..37626e9e847 100644 --- a/packages/core/src/extension/extensionManager.ts +++ b/packages/core/src/extension/extensionManager.ts @@ -1731,12 +1731,14 @@ export class ExtensionManager { if ( installMetadata.originSource === 'Claude' && installMetadata.marketplaceConfig && + installMetadata.pluginSourceKind !== 'extension-root' && !installMetadata.pluginName ) { const pluginName = await this.requestChoicePlugin( installMetadata.marketplaceConfig, ); installMetadata.pluginName = pluginName; + installMetadata.pluginSourceKind = 'marketplace-entry'; } if ( @@ -1812,6 +1814,7 @@ export class ExtensionManager { installMetadata.pluginName, installMetadata.networkPolicy, signal, + installMetadata.pluginSourceKind, ); signal?.throwIfAborted(); diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index d9f23be5f66..29d411c07b6 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -330,6 +330,7 @@ export async function checkForExtensionUpdate( installMetadata.pluginName, installMetadata.networkPolicy, signal, + installMetadata.pluginSourceKind, ); extensionDir = converted.extensionDir; if (extensionDir !== tempDir) { @@ -382,6 +383,7 @@ export async function checkForExtensionUpdate( installMetadata.pluginName, installMetadata.networkPolicy, signal, + installMetadata.pluginSourceKind, ); const extensionDir = converted.extensionDir; if (extensionDir !== tempDir) { diff --git a/packages/core/src/extension/marketplace.test.ts b/packages/core/src/extension/marketplace.test.ts index 9be144c6fda..dd40fdb017e 100644 --- a/packages/core/src/extension/marketplace.test.ts +++ b/packages/core/src/extension/marketplace.test.ts @@ -108,6 +108,18 @@ describe('parseInstallSource', () => { expect(result.source).toBe('https://github.com/owner/repo'); expect(result.type).toBe('git'); expect(result.pluginName).toBe('my-plugin'); + expect(result.pluginSourceKind).toBe('marketplace-entry'); + }); + + it('preserves an explicit root-plugin source kind', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + + const result = await parseInstallSource('owner/repo:alias', { + pluginSourceKind: 'extension-root', + }); + + expect(result.pluginName).toBe('alias'); + expect(result.pluginSourceKind).toBe('extension-root'); }); it('should handle owner/repo with dashes and underscores', async () => { diff --git a/packages/core/src/extension/marketplace.ts b/packages/core/src/extension/marketplace.ts index 5444b18090f..82141d8a78f 100644 --- a/packages/core/src/extension/marketplace.ts +++ b/packages/core/src/extension/marketplace.ts @@ -411,6 +411,7 @@ export async function parseInstallSource( source: string, options: { networkPolicy?: ExtensionInstallMetadata['networkPolicy']; + pluginSourceKind?: ExtensionInstallMetadata['pluginSourceKind']; } = {}, ): Promise { // Step 1: Parse source into repo and optional pluginName @@ -506,6 +507,11 @@ export async function parseInstallSource( if (options.networkPolicy) { installMetadata.networkPolicy = options.networkPolicy; } + const pluginSourceKind = + options.pluginSourceKind ?? (pluginName ? 'marketplace-entry' : undefined); + if (pluginSourceKind) { + installMetadata.pluginSourceKind = pluginSourceKind; + } return installMetadata; } diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index d13311cda5a..691618950a2 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -181,6 +181,7 @@ describe('discoverPlugins', () => { expect(pdf.installed).toBe(false); expect(pdf.homepage).toBe('https://example.com/pdf'); expect(pdf.installSource).toBe('anthropics/skills:pdf'); + expect(pdf.pluginSourceKind).toBe('marketplace-entry'); expect(discovered.find((p) => p.name === 'docx')!.installed).toBe(true); }); @@ -294,6 +295,19 @@ describe('discoverPlugins', () => { version: '1.0.0', source: { source: 'url', url: 'https://example.com/p.tgz' }, }, + { + name: 'https-root', + version: '1.0.0', + source: 'https://github.com/someone/root-plugin', + }, + { + name: 'url-github-root', + version: '1.0.0', + source: { + source: 'url', + url: 'https://github.com/someone/other-root-plugin', + }, + }, ]), ); @@ -305,9 +319,27 @@ describe('discoverPlugins', () => { expect(discovered.find((p) => p.name === 'gh-plugin')!.installSource).toBe( 'someone/repo:gh-plugin', ); + expect( + discovered.find((p) => p.name === 'gh-plugin')!.pluginSourceKind, + ).toBe('extension-root'); expect(discovered.find((p) => p.name === 'url-plugin')!.installSource).toBe( 'https://example.com/p.tgz', ); + expect( + discovered.find((p) => p.name === 'url-plugin')!.pluginSourceKind, + ).toBe('extension-root'); + expect(discovered.find((p) => p.name === 'https-root')!.installSource).toBe( + 'https://github.com/someone/root-plugin', + ); + expect( + discovered.find((p) => p.name === 'https-root')!.pluginSourceKind, + ).toBe('extension-root'); + expect( + discovered.find((p) => p.name === 'url-github-root')!.installSource, + ).toBe('https://github.com/someone/other-root-plugin'); + expect( + discovered.find((p) => p.name === 'url-github-root')!.pluginSourceKind, + ).toBe('extension-root'); }); it('rejects local-path sources from a remote (http) marketplace', async () => { diff --git a/packages/core/src/extension/sourceRegistry.ts b/packages/core/src/extension/sourceRegistry.ts index 6a3dfbcaa2f..e89e110df73 100644 --- a/packages/core/src/extension/sourceRegistry.ts +++ b/packages/core/src/extension/sourceRegistry.ts @@ -12,7 +12,10 @@ import { createDebugLogger } from '../utils/debugLogger.js'; import { redactUrlCredentials } from './redaction.js'; import { loadMarketplaceConfigFromSource } from './marketplace.js'; import { quarantineCorruptFile } from './corruptFile.js'; -import type { ExtensionInstallMetadata } from '../config/config.js'; +import type { + ExtensionInstallMetadata, + ExtensionPluginSourceKind, +} from '../config/config.js'; import type { ClaudeMarketplaceConfig, ClaudeMarketplacePluginConfig, @@ -65,6 +68,8 @@ export interface DiscoveredPlugin { components?: DiscoveredPluginComponents; /** Source string suitable for `parseInstallSource`. */ installSource: string; + /** Whether `pluginName` selects a marketplace entry or names a root plugin. */ + pluginSourceKind?: ExtensionPluginSourceKind; /** Whether an extension with this name is already installed. */ installed: boolean; } @@ -161,9 +166,15 @@ function isOwnerRepoShorthand(source: string): boolean { function resolveInstallSource( marketplace: ExtensionSource, plugin: ClaudeMarketplacePluginConfig, -): string { +): { + installSource: string; + pluginSourceKind: ExtensionPluginSourceKind; +} { if (marketplace.type !== 'http') { - return `${marketplace.source}:${plugin.name}`; + return { + installSource: `${marketplace.source}:${plugin.name}`, + pluginSourceKind: 'marketplace-entry', + }; } const src = plugin.source; if (typeof src === 'string') { @@ -173,12 +184,21 @@ function resolveInstallSource( debugLogger.warn( `Ignoring local path source "${src}" from remote marketplace "${marketplace.source}".`, ); - return plugin.name; + return { + installSource: plugin.name, + pluginSourceKind: 'extension-root', + }; } - return src.includes(':') ? src : `${src}:${plugin.name}`; + return { + installSource: src.includes(':') ? src : `${src}:${plugin.name}`, + pluginSourceKind: 'extension-root', + }; } if (src && src.source === 'github') { - return `${src.repo}:${plugin.name}`; + return { + installSource: `${src.repo}:${plugin.name}`, + pluginSourceKind: 'extension-root', + }; } if (src && src.source === 'url') { // Same local-path guard as the string-source branch above: a remote @@ -193,11 +213,20 @@ function resolveInstallSource( debugLogger.warn( `Ignoring local path source "${src.url}" from remote marketplace "${marketplace.source}".`, ); - return plugin.name; + return { + installSource: plugin.name, + pluginSourceKind: 'extension-root', + }; } - return src.url; + return { + installSource: src.url, + pluginSourceKind: 'extension-root', + }; } - return plugin.name; + return { + installSource: plugin.name, + pluginSourceKind: 'extension-root', + }; } /** @@ -221,24 +250,27 @@ function pluginsFromConfig( config: ClaudeMarketplaceConfig, installedNames: ReadonlySet, ): DiscoveredPlugin[] { - return (config.plugins ?? []).map((plugin) => ({ - marketplaceName: sanitizeDisplay(config.name || marketplace.name), - name: sanitizeDisplay(plugin.name), - description: sanitizeDisplay(plugin.description), - // `version` and `lastUpdated` render in the pre-consent Discover detail via - // `t()` (no escaping), so they need the same scrubbing as the other - // untrusted display fields. `category` has no sink today but is wrapped for - // consistency / future-proofing. - version: sanitizeDisplay(plugin.version), - author: sanitizeDisplay(plugin.author?.name), - homepage: sanitizeDisplay(plugin.homepage), - category: sanitizeDisplay(plugin.category), - lastUpdated: sanitizeDisplay(pluginLastUpdated(plugin)), - installs: pluginInstalls(plugin), - components: pluginComponents(plugin), - installSource: resolveInstallSource(marketplace, plugin), - installed: installedNames.has(plugin.name), - })); + return (config.plugins ?? []).map((plugin) => { + const installTarget = resolveInstallSource(marketplace, plugin); + return { + marketplaceName: sanitizeDisplay(config.name || marketplace.name), + name: sanitizeDisplay(plugin.name), + description: sanitizeDisplay(plugin.description), + // `version` and `lastUpdated` render in the pre-consent Discover detail + // via `t()` (no escaping), so they need the same scrubbing as the other + // untrusted display fields. `category` has no sink today but is wrapped + // for consistency / future-proofing. + version: sanitizeDisplay(plugin.version), + author: sanitizeDisplay(plugin.author?.name), + homepage: sanitizeDisplay(plugin.homepage), + category: sanitizeDisplay(plugin.category), + lastUpdated: sanitizeDisplay(pluginLastUpdated(plugin)), + installs: pluginInstalls(plugin), + components: pluginComponents(plugin), + ...installTarget, + installed: installedNames.has(plugin.name), + }; + }); } /** From 089e34f254f0c831027f3a535e574735a24bac1d Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Fri, 7 Aug 2026 17:42:39 +0800 Subject: [PATCH 02/19] fix(extensions): address dual-manifest review feedback --- .../src/extension/claude-converter.test.ts | 60 +++++ .../core/src/extension/claude-converter.ts | 6 +- .../src/extension/extension-converter.test.ts | 225 ++++++++++++++++-- .../core/src/extension/extension-converter.ts | 126 ++++++++-- .../src/extension/extensionManager.test.ts | 221 +++++++++++++++++ .../core/src/extension/extensionManager.ts | 7 +- packages/core/src/extension/github.test.ts | 66 +++++ 7 files changed, 679 insertions(+), 32 deletions(-) diff --git a/packages/core/src/extension/claude-converter.test.ts b/packages/core/src/extension/claude-converter.test.ts index 91f4effff22..cfff2e4cde7 100644 --- a/packages/core/src/extension/claude-converter.test.ts +++ b/packages/core/src/extension/claude-converter.test.ts @@ -279,6 +279,66 @@ describe('convertClaudePluginPackage', () => { } }); + it('treats a marketplace entry without source as the marketplace root', async () => { + const marketplaceDir = path.join(testDir, '.claude-plugin'); + fs.mkdirSync(marketplaceDir, { recursive: true }); + fs.writeFileSync( + path.join(marketplaceDir, 'marketplace.json'), + JSON.stringify({ + name: 'root-marketplace', + owner: { name: 'Owner' }, + plugins: [{ name: 'root-plugin', version: '1.0.0' }], + }), + ); + + const result = await convertClaudePluginPackage(testDir, 'root-plugin'); + try { + expect(result.config.name).toBe('root-plugin'); + expect(result.config.version).toBe('1.0.0'); + } finally { + fs.rmSync(result.convertedDir, { recursive: true, force: true }); + } + }); + + it('cleans the marketplace staging directory when conversion fails', async () => { + const marketplaceDir = path.join(testDir, '.claude-plugin'); + fs.mkdirSync(marketplaceDir, { recursive: true }); + fs.writeFileSync( + path.join(marketplaceDir, 'marketplace.json'), + JSON.stringify({ + name: 'strict-marketplace', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'strict-root', + version: '1.0.0', + source: './', + strict: true, + }, + ], + }), + ); + const createTmpDir = ExtensionStorage.createTmpDir.bind(ExtensionStorage); + const tempDirs: string[] = []; + const createTmpDirSpy = vi + .spyOn(ExtensionStorage, 'createTmpDir') + .mockImplementation(async () => { + const tempDir = await createTmpDir(); + tempDirs.push(tempDir); + return tempDir; + }); + + try { + await expect( + convertClaudePluginPackage(testDir, 'strict-root'), + ).rejects.toThrow('Strict mode requires plugin.json'); + expect(tempDirs).toHaveLength(1); + expect(fs.existsSync(tempDirs[0])).toBe(false); + } finally { + createTmpDirSpy.mockRestore(); + } + }); + it('should only collect specified skills when config provides explicit list', async () => { // Setup: Create a plugin source with multiple skills const pluginSourceDir = path.join(testDir, 'plugin-source'); diff --git a/packages/core/src/extension/claude-converter.ts b/packages/core/src/extension/claude-converter.ts index 1f2c6c6cba8..e7cbd15e707 100644 --- a/packages/core/src/extension/claude-converter.ts +++ b/packages/core/src/extension/claude-converter.ts @@ -102,7 +102,7 @@ export type ClaudePluginSource = }; export interface ClaudeMarketplacePluginConfig extends ClaudePluginConfig { - source: string | ClaudePluginSource; + source?: string | ClaudePluginSource; category?: string; strict?: boolean; tags?: string[]; @@ -1040,6 +1040,10 @@ async function resolvePluginSource( signal?.throwIfAborted(); const source = pluginConfig.source; + // A marketplace entry without `source` describes a plugin at the + // marketplace root. + if (source === undefined || source === null) return marketplaceDir; + // Handle string source (relative path or URL) if (typeof source === 'string') { // Check if it's a URL (scheme is case-insensitive, e.g. HTTPS://) diff --git a/packages/core/src/extension/extension-converter.test.ts b/packages/core/src/extension/extension-converter.test.ts index 030742db120..3052558f8b7 100644 --- a/packages/core/src/extension/extension-converter.test.ts +++ b/packages/core/src/extension/extension-converter.test.ts @@ -4,13 +4,14 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { convertGeminiOrClaudeExtension } from './extension-converter.js'; import { ExtensionManager, type ExtensionConfig } from './extensionManager.js'; import { ExtensionStore } from './extension-store.js'; +import { ExtensionStorage } from './storage.js'; describe('convertGeminiOrClaudeExtension', () => { let extensionDir: string; @@ -82,6 +83,46 @@ describe('convertGeminiOrClaudeExtension', () => { pluginSourceKind: undefined, includeMarketplace: true, }, + { + installKind: 'standalone without marketplace', + pluginName: undefined, + marketplacePluginName: 'ponytail', + marketplaceSource: './', + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: undefined, + includeMarketplace: false, + }, + { + installKind: 'sourceless marketplace root', + pluginName: 'ponytail', + marketplacePluginName: 'ponytail', + marketplaceSource: undefined, + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: 'marketplace-entry' as const, + includeMarketplace: true, + }, + { + installKind: 'explicit matching extension root', + pluginName: 'ponytail', + marketplacePluginName: 'ponytail', + marketplaceSource: './', + marketplaceHooks: './hooks/marketplace-hooks.json', + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: 'extension-root' as const, + includeMarketplace: true, + }, + { + installKind: 'explicit matching marketplace root', + pluginName: 'ponytail', + marketplacePluginName: 'ponytail', + marketplaceSource: './', + marketplaceHooks: './hooks/marketplace-hooks.json', + expectedHookPath: 'scripts/marketplace-session-start.sh', + pluginSourceKind: 'marketplace-entry' as const, + includeMarketplace: true, + }, ])( 'keeps Gemini resources and imports only selected root Claude hooks for a dual-manifest $installKind extension', async ({ @@ -207,6 +248,7 @@ describe('convertGeminiOrClaudeExtension', () => { ) as ExtensionConfig; expect(converted.originSource).toBe('Gemini'); + expect(converted.requiresClaudeFileAdaptation).toBe(true); expect(config.name).toBe('ponytail'); expect(config.contextFileName).toBe('AGENTS.md'); expect(config.settings).toEqual([ @@ -256,13 +298,12 @@ describe('convertGeminiOrClaudeExtension', () => { }); expect(fs.existsSync(extensionDir)).toBe(false); - expect( - ( - installedConfig.hooks?.['SessionStart']?.[0].hooks?.[0] as { - command?: string; - } - )?.command, - ).toBe( + const installedCommand = ( + installedConfig.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command; + expect(installedCommand && path.normalize(installedCommand)).toBe( expectedHookPath ? path.join(installedDir, expectedHookPath) : undefined, @@ -457,12 +498,168 @@ describe('convertGeminiOrClaudeExtension', () => { const installedConfig = manager.loadExtensionConfig({ extensionDir: installedDir, }); + const installedCommand = ( + installedConfig.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command; + expect(installedCommand && path.normalize(installedCommand)).toBe( + path.join(installedDir, 'scripts', 'start.sh'), + ); + }); + + it('merges conventional Gemini hooks with Claude hooks by event', async () => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ name: 'dual-hooks', version: '1.0.0' }), + ); + const manifestDir = path.join(extensionDir, '.claude-plugin'); + const hooksDir = path.join(extensionDir, 'hooks'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.writeFileSync( + path.join(manifestDir, 'plugin.json'), + JSON.stringify({ + name: 'dual-hooks', + version: '1.0.0', + hooks: './hooks/claude-hooks.json', + }), + ); + fs.writeFileSync( + path.join(hooksDir, 'hooks.json'), + JSON.stringify({ + hooks: { + PreToolUse: [ + { hooks: [{ type: 'command', command: 'gemini-pre.sh' }] }, + ], + SessionStart: [ + { hooks: [{ type: 'command', command: 'gemini-start.sh' }] }, + ], + }, + }), + ); + fs.writeFileSync( + path.join(hooksDir, 'claude-hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: 'claude-start.sh' }] }, + ], + }, + }), + ); + + const converted = await convertGeminiOrClaudeExtension(extensionDir); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(config.hooks?.['PreToolUse']).toHaveLength(1); expect( - ( - installedConfig.hooks?.['SessionStart']?.[0].hooks?.[0] as { - command?: string; - } - )?.command, - ).toBe(path.join(installedDir, 'scripts', 'start.sh')); + config.hooks?.['SessionStart']?.map( + (definition) => + (definition.hooks?.[0] as { command?: string })?.command, + ), + ).toEqual(['gemini-start.sh', 'claude-start.sh']); + }); + + it('keeps a valid Gemini conversion when Claude metadata is malformed', async () => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ name: 'valid-gemini', version: '1.0.0' }), + ); + const manifestDir = path.join(extensionDir, '.claude-plugin'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.writeFileSync(path.join(manifestDir, 'plugin.json'), '{'); + + const converted = await convertGeminiOrClaudeExtension(extensionDir); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(converted.originSource).toBe('Gemini'); + expect(converted.requiresClaudeFileAdaptation).toBe(false); + expect(config.name).toBe('valid-gemini'); + }); + + it('removes the temporary Claude conversion after a successful merge', async () => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ name: 'cleanup-success', version: '1.0.0' }), + ); + const manifestDir = path.join(extensionDir, '.claude-plugin'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.writeFileSync( + path.join(manifestDir, 'plugin.json'), + JSON.stringify({ name: 'cleanup-success', version: '1.0.0' }), + ); + const createTmpDir = ExtensionStorage.createTmpDir.bind(ExtensionStorage); + const tempDirs: string[] = []; + const createTmpDirSpy = vi + .spyOn(ExtensionStorage, 'createTmpDir') + .mockImplementation(async () => { + const tempDir = await createTmpDir(); + tempDirs.push(tempDir); + return tempDir; + }); + + try { + const converted = await convertGeminiOrClaudeExtension(extensionDir); + convertedDir = converted.extensionDir; + expect(tempDirs).toHaveLength(2); + expect(converted.extensionDir).toBe(tempDirs[0]); + expect(fs.existsSync(tempDirs[0])).toBe(true); + expect(fs.existsSync(tempDirs[1])).toBe(false); + } finally { + createTmpDirSpy.mockRestore(); + } + }); + + it('removes both conversions when a merge is aborted before writing', async () => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ name: 'cleanup-abort', version: '1.0.0' }), + ); + const manifestDir = path.join(extensionDir, '.claude-plugin'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.writeFileSync( + path.join(manifestDir, 'plugin.json'), + JSON.stringify({ name: 'cleanup-abort', version: '1.0.0' }), + ); + const controller = new AbortController(); + const reason = new Error('conversion expired'); + const createTmpDir = ExtensionStorage.createTmpDir.bind(ExtensionStorage); + const tempDirs: string[] = []; + const createTmpDirSpy = vi + .spyOn(ExtensionStorage, 'createTmpDir') + .mockImplementation(async () => { + const tempDir = await createTmpDir(); + tempDirs.push(tempDir); + if (tempDirs.length === 2) controller.abort(reason); + return tempDir; + }); + + try { + await expect( + convertGeminiOrClaudeExtension( + extensionDir, + undefined, + undefined, + controller.signal, + ), + ).rejects.toBe(reason); + expect(tempDirs).toHaveLength(2); + expect(tempDirs.every((tempDir) => !fs.existsSync(tempDir))).toBe(true); + } finally { + createTmpDirSpy.mockRestore(); + } }); }); diff --git a/packages/core/src/extension/extension-converter.ts b/packages/core/src/extension/extension-converter.ts index 42a19e0098f..6b62495b244 100644 --- a/packages/core/src/extension/extension-converter.ts +++ b/packages/core/src/extension/extension-converter.ts @@ -16,11 +16,15 @@ import { convertClaudePluginPackage, convertClaudePluginStandalone, } from './claude-converter.js'; +import type { ExtensionConfig } from './extensionManager.js'; import type { ExtensionNetworkPolicy, ExtensionOriginSource, ExtensionPluginSourceKind, } from '../config/config.js'; +import { createDebugLogger } from '../utils/debugLogger.js'; + +const debugLogger = createDebugLogger('EXTENSION_CONVERTER'); export const SUPPORTED_EXTENSION_MANIFESTS = [ EXTENSIONS_CONFIG_FILENAME, @@ -74,11 +78,17 @@ function selectedMarketplacePluginLocation( const selectedPlugin = ( marketplace as { plugins: Array> } ).plugins.find((plugin) => plugin['name'] === pluginName); - if (typeof selectedPlugin?.['source'] !== 'string') { + if (!selectedPlugin) { return 'other'; } - return path.resolve(path.join(extensionDir, selectedPlugin['source'])) === + const source = selectedPlugin['source']; + // Claude marketplaces allow an entry without `source`; that entry refers + // to the marketplace root itself. + if (source === undefined || source === null) return 'root'; + if (typeof source !== 'string') return 'other'; + + return path.resolve(path.join(extensionDir, source)) === path.resolve(extensionDir) ? 'root' : 'other'; @@ -87,16 +97,75 @@ function selectedMarketplacePluginLocation( } } +type ExtensionHooks = NonNullable; + +function loadConventionalHooks( + convertedDir: string, +): ExtensionHooks | undefined { + const hooksPath = path.join(convertedDir, 'hooks', 'hooks.json'); + if (!fs.existsSync(hooksPath) || !realPathWithin(hooksPath, convertedDir)) { + return undefined; + } + + try { + const parsed: unknown = JSON.parse(fs.readFileSync(hooksPath, 'utf-8')); + if (typeof parsed !== 'object' || parsed === null) return undefined; + const hooks = (parsed as { hooks?: unknown }).hooks ?? parsed; + return typeof hooks === 'object' && hooks !== null + ? (hooks as ExtensionHooks) + : undefined; + } catch (error) { + debugLogger.warn( + `Failed to parse hooks file ${hooksPath}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + return undefined; + } +} + +function mergeHooks( + ...sources: Array +): ExtensionHooks | undefined { + const merged: ExtensionHooks = {}; + for (const source of sources) { + if (!source) continue; + for (const [event, definitions] of Object.entries(source)) { + if (!Array.isArray(definitions)) continue; + const existing = merged[event as keyof ExtensionHooks] ?? []; + const serialized = new Set( + existing.map((entry) => JSON.stringify(entry)), + ); + const uniqueDefinitions = definitions.filter((entry) => { + const key = JSON.stringify(entry); + if (serialized.has(key)) return false; + serialized.add(key); + return true; + }); + merged[event as keyof ExtensionHooks] = [ + ...existing, + ...uniqueDefinitions, + ]; + } + } + return Object.keys(merged).length > 0 ? merged : undefined; +} + export async function convertGeminiOrClaudeExtension( extensionDir: string, pluginName?: string, networkPolicy?: ExtensionNetworkPolicy, signal?: AbortSignal, pluginSourceKind?: ExtensionPluginSourceKind, -): Promise<{ extensionDir: string; originSource: ExtensionOriginSource }> { +): Promise<{ + extensionDir: string; + originSource: ExtensionOriginSource; + requiresClaudeFileAdaptation: boolean; +}> { signal?.throwIfAborted(); let newExtensionDir = extensionDir; let originSource: ExtensionOriginSource = 'QwenCode'; + let requiresClaudeFileAdaptation = false; const configFilePath = path.join( extensionDir, SUPPORTED_EXTENSION_MANIFESTS[0], @@ -150,18 +219,42 @@ export async function convertGeminiOrClaudeExtension( | undefined; try { signal?.throwIfAborted(); - claudeConversion = rootMarketplacePluginName - ? await convertClaudePluginPackage( - extensionDir, - rootMarketplacePluginName, - networkPolicy, - signal, - true, - ) - : await convertClaudePluginStandalone(extensionDir, true); + try { + claudeConversion = rootMarketplacePluginName + ? await convertClaudePluginPackage( + extensionDir, + rootMarketplacePluginName, + networkPolicy, + signal, + true, + ) + : await convertClaudePluginStandalone(extensionDir, true); + } catch (error) { + signal?.throwIfAborted(); + debugLogger.warn( + `Failed to import Claude plugin metadata; keeping the Gemini extension: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + } + + if (!claudeConversion) { + newExtensionDir = geminiConversion.convertedDir; + originSource = 'Gemini'; + return { + extensionDir: newExtensionDir, + originSource, + requiresClaudeFileAdaptation, + }; + } + + const geminiHooks = + geminiConversion.config.hooks ?? + loadConventionalHooks(geminiConversion.convertedDir); + const claudeHooks = claudeConversion.config.hooks; const mergedConfig = { ...geminiConversion.config, - hooks: claudeConversion.config.hooks ?? geminiConversion.config.hooks, + hooks: mergeHooks(geminiHooks, claudeHooks), }; signal?.throwIfAborted(); fs.writeFileSync( @@ -171,6 +264,7 @@ export async function convertGeminiOrClaudeExtension( ); newExtensionDir = geminiConversion.convertedDir; originSource = 'Gemini'; + requiresClaudeFileAdaptation = Boolean(claudeHooks); } catch (error) { removeConvertedDirectory(geminiConversion.convertedDir); throw error; @@ -200,5 +294,9 @@ export async function convertGeminiOrClaudeExtension( originSource = 'Claude'; } signal?.throwIfAborted(); - return { extensionDir: newExtensionDir, originSource }; + return { + extensionDir: newExtensionDir, + originSource, + requiresClaudeFileAdaptation, + }; } diff --git a/packages/core/src/extension/extensionManager.test.ts b/packages/core/src/extension/extensionManager.test.ts index 3fc02f11aa9..679822e0f26 100644 --- a/packages/core/src/extension/extensionManager.test.ts +++ b/packages/core/src/extension/extensionManager.test.ts @@ -30,6 +30,7 @@ import { import type { MCPServerConfig, ExtensionInstallMetadata } from '../index.js'; import { ExtensionStore } from './extension-store.js'; import { ExtensionPreferencesStore } from './extensionPreferences.js'; +import { parseInstallSource } from './marketplace.js'; const mockGit = { clone: vi.fn(), @@ -245,6 +246,132 @@ describe('extension tests', () => { expect(installed.name).toBe('direct-root-extension'); }); + it('preserves the CLI marketplace-entry kind through a dual-manifest install', async () => { + const sourceDir = path.join(tempWorkspaceDir, 'cli-dual-source'); + const manifestDir = path.join(sourceDir, '.claude-plugin'); + const hooksDir = path.join(sourceDir, 'hooks'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.writeFileSync( + path.join(sourceDir, 'gemini-extension.json'), + JSON.stringify({ + name: 'cli-dual', + version: '1.0.0', + contextFileName: 'AGENTS.md', + settings: [ + { + name: 'Mode', + envVar: 'CLI_DUAL_MODE', + description: 'CLI dual mode', + }, + ], + }), + ); + fs.writeFileSync(path.join(sourceDir, 'AGENTS.md'), '# CLI dual'); + const marketplaceConfig = { + name: 'cli-dual-marketplace', + owner: { name: 'Owner', email: 'owner@example.com' }, + plugins: [{ name: 'cli-dual', version: '1.0.0', source: './' }], + }; + fs.writeFileSync( + path.join(manifestDir, 'marketplace.json'), + JSON.stringify(marketplaceConfig), + ); + fs.writeFileSync( + path.join(manifestDir, 'plugin.json'), + JSON.stringify({ + name: 'cli-dual', + version: '1.0.0', + hooks: './hooks/hooks.json', + }), + ); + fs.writeFileSync( + path.join(hooksDir, 'hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { hooks: [{ type: 'command', command: 'session-start.sh' }] }, + ], + }, + }), + ); + + const installMetadata = await parseInstallSource(`${sourceDir}:cli-dual`); + expect(installMetadata.pluginSourceKind).toBe('marketplace-entry'); + const manager = createExtensionManager(); + const installed = await manager.installExtension( + installMetadata, + async () => {}, + async () => 'enabled', + ); + const persistedMetadata = JSON.parse( + fs.readFileSync( + path.join(installed.path, INSTALL_METADATA_FILENAME), + 'utf-8', + ), + ) as ExtensionInstallMetadata; + + expect(installed.config.contextFileName).toBe('AGENTS.md'); + expect(installed.config.settings).toHaveLength(1); + expect(installed.hooks?.['SessionStart']).toHaveLength(1); + expect(persistedMetadata.pluginSourceKind).toBe('marketplace-entry'); + expect(persistedMetadata.pluginName).toBe('cli-dual'); + }); + + it('persists marketplace-entry after an interactive plugin choice', async () => { + const sourceDir = path.join(tempWorkspaceDir, 'prompt-marketplace'); + const manifestDir = path.join(sourceDir, '.claude-plugin'); + const childManifestDir = path.join( + sourceDir, + 'plugins', + 'chosen-plugin', + '.claude-plugin', + ); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.mkdirSync(childManifestDir, { recursive: true }); + const marketplaceConfig = { + name: 'prompt-marketplace', + owner: { name: 'Owner', email: 'owner@example.com' }, + plugins: [ + { + name: 'chosen-plugin', + version: '2.0.0', + source: './plugins/chosen-plugin', + }, + ], + }; + fs.writeFileSync( + path.join(manifestDir, 'marketplace.json'), + JSON.stringify(marketplaceConfig), + ); + fs.writeFileSync( + path.join(childManifestDir, 'plugin.json'), + JSON.stringify({ name: 'chosen-plugin', version: '2.0.0' }), + ); + const requestChoicePlugin = vi.fn(async () => 'chosen-plugin'); + const manager = createExtensionManager({ requestChoicePlugin }); + + const installed = await manager.installExtension( + { + type: 'local', + source: sourceDir, + originSource: 'Claude', + marketplaceConfig, + }, + async () => {}, + ); + const persistedMetadata = JSON.parse( + fs.readFileSync( + path.join(installed.path, INSTALL_METADATA_FILENAME), + 'utf-8', + ), + ) as ExtensionInstallMetadata; + + expect(requestChoicePlugin).toHaveBeenCalledOnce(); + expect(persistedMetadata.pluginName).toBe('chosen-plugin'); + expect(persistedMetadata.pluginSourceKind).toBe('marketplace-entry'); + }); + it('commits workspace initial activation with the installed artifact', async () => { const archivePath = path.join(tempWorkspaceDir, 'workspace-ext.zip'); fs.writeFileSync(archivePath, 'archive'); @@ -481,6 +608,100 @@ describe('extension tests', () => { } }); + it('adapts Claude hook files in a dual-manifest Gemini extension', async () => { + const archivePath = path.join(tempWorkspaceDir, 'dual-manifest.zip'); + fs.writeFileSync(archivePath, 'archive'); + mockExtractArchiveFile.mockImplementation( + async (_source: string, destination: string) => { + fs.mkdirSync(destination, { recursive: true }); + fs.writeFileSync( + path.join(destination, 'gemini-extension.json'), + JSON.stringify({ + name: 'dual-manifest', + version: '1.0.0', + settings: [ + { + name: 'Mode', + envVar: 'DUAL_MODE', + description: 'Dual mode', + }, + ], + }), + ); + const manifestDir = path.join(destination, '.claude-plugin'); + const hooksDir = path.join(destination, 'hooks'); + const scriptsDir = path.join(destination, 'scripts'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.mkdirSync(hooksDir, { recursive: true }); + fs.mkdirSync(scriptsDir, { recursive: true }); + fs.writeFileSync( + path.join(manifestDir, 'plugin.json'), + JSON.stringify({ + name: 'dual-manifest', + version: '1.0.0', + hooks: './hooks/hooks.json', + }), + ); + fs.writeFileSync( + path.join(hooksDir, 'hooks.json'), + JSON.stringify({ + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: + '${CLAUDE_PLUGIN_ROOT}/scripts/session-start.sh', + }, + ], + }, + ], + }, + }), + ); + fs.writeFileSync( + path.join(scriptsDir, 'session-start.sh'), + 'jq \'.message.content | map(select(.type == "text"))\' ~/.claude/transcript\n', + ); + }, + ); + const manager = createExtensionManager(); + const prepared = await manager.prepareExtensionInstall({ + installMetadata: { type: 'local', source: archivePath }, + initialActivation: { scope: 'user' }, + requestConsent: async () => {}, + requestSetting: async () => 'enabled', + }); + + try { + const stagedConfig = JSON.parse( + fs.readFileSync( + path.join(prepared.stagingDirectory, EXTENSIONS_CONFIG_FILENAME), + 'utf-8', + ), + ) as ExtensionConfig; + const stagedMetadata = JSON.parse( + fs.readFileSync( + path.join(prepared.stagingDirectory, INSTALL_METADATA_FILENAME), + 'utf-8', + ), + ) as ExtensionInstallMetadata; + const script = fs.readFileSync( + path.join(prepared.stagingDirectory, 'scripts', 'session-start.sh'), + 'utf-8', + ); + + expect(stagedMetadata.originSource).toBe('Gemini'); + expect(stagedConfig.settings).toHaveLength(1); + expect(stagedConfig.hooks?.['SessionStart']).toHaveLength(1); + expect(script).toContain('.message.parts | map(select(has("text")))'); + expect(script).toContain('~/.qwen/transcript'); + } finally { + await manager.disposePreparedExtension(prepared); + } + }); + it('does not report a temp cleanup warning when an immediate retry succeeds', async () => { const archivePath = path.join(tempWorkspaceDir, 'cleanup-warning.zip'); fs.writeFileSync(archivePath, 'archive'); diff --git a/packages/core/src/extension/extensionManager.ts b/packages/core/src/extension/extensionManager.ts index 37626e9e847..c557e7e57fd 100644 --- a/packages/core/src/extension/extensionManager.ts +++ b/packages/core/src/extension/extensionManager.ts @@ -1808,7 +1808,7 @@ export class ExtensionManager { signal?.throwIfAborted(); try { const sourceBeforeConversion = localSourcePath; - const { extensionDir, originSource } = + const { extensionDir, originSource, requiresClaudeFileAdaptation } = await convertGeminiOrClaudeExtension( sourceBeforeConversion, installMetadata.pluginName, @@ -1954,8 +1954,9 @@ export class ExtensionManager { : null; if ( - (originSource === 'Claude' && fs.existsSync(hooksDir)) || - (originSource === 'Claude' && + ((originSource === 'Claude' || requiresClaudeFileAdaptation) && + fs.existsSync(hooksDir)) || + ((originSource === 'Claude' || requiresClaudeFileAdaptation) && configHooksPath && fs.existsSync(configHooksPath)) ) { diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index 8cdf5643c58..1f062667d8c 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -770,6 +770,72 @@ describe('git extension helpers', () => { } }); + it('forwards extension-root kind when checking a local archive update', async () => { + const tempDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'local-root-alias-update-test-'), + ); + try { + const archivePath = path.join(tempDir, 'root-alias.zip'); + const archive = await createZipBuffer(tempDir, [ + { + name: '.claude-plugin/plugin.json', + content: JSON.stringify({ + name: 'root-plugin', + version: '1.0.0', + }), + }, + { + name: '.claude-plugin/marketplace.json', + content: JSON.stringify({ + name: 'root-marketplace', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'root-alias', + version: '2.0.0', + source: './plugins/root-alias', + }, + ], + }), + }, + { + name: 'plugins/root-alias/.claude-plugin/plugin.json', + content: JSON.stringify({ + name: 'child-plugin', + version: '2.0.0', + }), + }, + ]); + await fs.writeFile(archivePath, archive); + const extension = createExtension({ + version: '1.0.0', + installMetadata: { + type: 'local', + source: archivePath, + pluginName: 'root-alias', + pluginSourceKind: 'extension-root', + }, + }); + const mockManager = { + loadExtensionConfig: vi.fn( + ({ extensionDir }: { extensionDir: string }) => + JSON.parse( + fsSync.readFileSync( + path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME), + 'utf-8', + ), + ), + ), + } as unknown as ExtensionManager; + + const result = await checkForExtensionUpdate(extension, mockManager); + + expect(result).toBe(ExtensionUpdateState.UP_TO_DATE); + } finally { + await fs.rm(tempDir, { recursive: true, force: true }); + } + }); + it('should return UPDATE_AVAILABLE for local archive extension with different version', async () => { const tempDir = await fs.mkdtemp( path.join(os.tmpdir(), 'local-archive-update-test-'), From 9c38138c2db8b4ee49d78c15296305a713517f92 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Sat, 8 Aug 2026 02:56:46 +0800 Subject: [PATCH 03/19] fix(extensions): address follow-up review blockers --- .../src/serve/routes/workspace-extensions.ts | 16 +++-- packages/cli/src/serve/server.test.ts | 12 +++- .../src/extension/extension-converter.test.ts | 52 +++++++++++++++ .../core/src/extension/extension-converter.ts | 1 + packages/core/src/extension/github.test.ts | 66 +++++++++++++++++++ packages/core/src/extension/github.ts | 26 ++++---- 6 files changed, 153 insertions(+), 20 deletions(-) diff --git a/packages/cli/src/serve/routes/workspace-extensions.ts b/packages/cli/src/serve/routes/workspace-extensions.ts index 6f16eb15053..f77684bd32e 100644 --- a/packages/cli/src/serve/routes/workspace-extensions.ts +++ b/packages/cli/src/serve/routes/workspace-extensions.ts @@ -347,13 +347,15 @@ export function registerWorkspaceExtensionRoutes( plugins: marketplace.plugins.map((plugin) => ({ name: plugin.name, ...(plugin.description ? { description: plugin.description } : {}), - source: redactExtensionDisplaySource( - typeof plugin.source === 'string' - ? plugin.source - : plugin.source.source === 'github' - ? plugin.source.repo - : plugin.source.url, - ), + source: plugin.source + ? redactExtensionDisplaySource( + typeof plugin.source === 'string' + ? plugin.source + : plugin.source.source === 'github' + ? plugin.source.repo + : plugin.source.url, + ) + : '.', ...(plugin.category ? { category: plugin.category } : {}), ...(plugin.tags ? { tags: plugin.tags } : {}), })), diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index f933a46aec7..e6d04bebd10 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -5163,7 +5163,8 @@ describe('createServeApp', () => { owner: { name: string; email: string }; plugins: Array<{ name: string; - source: string; + source?: string; + description?: string; category?: string; tags?: string[]; }>; @@ -5180,6 +5181,10 @@ describe('createServeApp', () => { category: 'tools', tags: ['example'], }, + { + name: 'root-plugin', + description: 'Plugin at the marketplace root', + }, ], }); return testExtension('example-plugin'); @@ -5222,6 +5227,11 @@ describe('createServeApp', () => { category: 'tools', tags: ['example'], }, + { + name: 'root-plugin', + description: 'Plugin at the marketplace root', + source: '.', + }, ], }, }); diff --git a/packages/core/src/extension/extension-converter.test.ts b/packages/core/src/extension/extension-converter.test.ts index 3052558f8b7..e6bc05b2a5d 100644 --- a/packages/core/src/extension/extension-converter.test.ts +++ b/packages/core/src/extension/extension-converter.test.ts @@ -93,6 +93,16 @@ describe('convertGeminiOrClaudeExtension', () => { pluginSourceKind: undefined, includeMarketplace: false, }, + { + installKind: 'named root without marketplace', + pluginName: 'ponytail', + marketplacePluginName: 'ponytail', + marketplaceSource: './', + marketplaceHooks: undefined, + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: 'marketplace-entry' as const, + includeMarketplace: false, + }, { installKind: 'sourceless marketplace root', pluginName: 'ponytail', @@ -311,6 +321,48 @@ describe('convertGeminiOrClaudeExtension', () => { }, ); + it.each([ + { + format: 'Qwen', + manifest: 'qwen-extension.json', + expectedOrigin: 'QwenCode' as const, + }, + { + format: 'Gemini', + manifest: 'gemini-extension.json', + expectedOrigin: 'Gemini' as const, + }, + ])( + 'keeps a classic $format root extension when a named install has no marketplace', + async ({ manifest, expectedOrigin }) => { + fs.writeFileSync( + path.join(extensionDir, manifest), + JSON.stringify({ name: 'classic-root', version: '1.0.0' }), + ); + + const converted = await convertGeminiOrClaudeExtension( + extensionDir, + 'classic-root', + undefined, + undefined, + 'marketplace-entry', + ); + if (converted.extensionDir !== extensionDir) { + convertedDir = converted.extensionDir; + } + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(converted.originSource).toBe(expectedOrigin); + expect(config.name).toBe('classic-root'); + expect(config.version).toBe('1.0.0'); + }, + ); + it('treats pluginName as an alias for an explicit Claude extension-root install', async () => { const claudeDir = path.join(extensionDir, '.claude-plugin'); const hooksDir = path.join(extensionDir, 'hooks'); diff --git a/packages/core/src/extension/extension-converter.ts b/packages/core/src/extension/extension-converter.ts index 6b62495b244..e9fbc6d6335 100644 --- a/packages/core/src/extension/extension-converter.ts +++ b/packages/core/src/extension/extension-converter.ts @@ -198,6 +198,7 @@ export async function convertGeminiOrClaudeExtension( if ( isExplicitMarketplaceEntry && pluginName && + marketplaceLocation !== 'missing-marketplace' && !selectedMarketplaceEntryUsesRoot ) { newExtensionDir = ( diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index 1f062667d8c..1c7deab2a92 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -721,6 +721,72 @@ describe('git extension helpers', () => { expect(result).toBe(ExtensionUpdateState.NOT_UPDATABLE); }); + it('checks a selected marketplace entry when its local source is a directory', async () => { + const sourceDir = await fs.mkdtemp( + path.join(os.tmpdir(), 'local-marketplace-update-test-'), + ); + try { + await fs.writeFile( + path.join(sourceDir, EXTENSIONS_CONFIG_FILENAME), + JSON.stringify({ name: 'marketplace-root', version: '1.0.0' }), + ); + const marketplaceDir = path.join(sourceDir, '.claude-plugin'); + const pluginDir = path.join(sourceDir, 'plugins', 'selected'); + await fs.mkdir(marketplaceDir, { recursive: true }); + await fs.mkdir(path.join(pluginDir, '.claude-plugin'), { + recursive: true, + }); + await fs.writeFile( + path.join(marketplaceDir, 'marketplace.json'), + JSON.stringify({ + name: 'marketplace-root', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'selected', + source: './plugins/selected', + }, + ], + }), + ); + await fs.writeFile( + path.join(pluginDir, '.claude-plugin', 'plugin.json'), + JSON.stringify({ name: 'selected', version: '2.0.0' }), + ); + const mockManager = { + loadExtensionConfig: vi.fn( + ({ extensionDir }: { extensionDir: string }) => + JSON.parse( + fsSync.readFileSync( + path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME), + 'utf-8', + ), + ), + ), + } as unknown as ExtensionManager; + const installMetadata = { + type: 'local' as const, + source: sourceDir, + pluginName: 'selected', + pluginSourceKind: 'marketplace-entry' as const, + }; + + const unchanged = await checkForExtensionUpdate( + createExtension({ version: '2.0.0', installMetadata }), + mockManager, + ); + const outdated = await checkForExtensionUpdate( + createExtension({ version: '1.0.0', installMetadata }), + mockManager, + ); + + expect(unchanged).toBe(ExtensionUpdateState.UP_TO_DATE); + expect(outdated).toBe(ExtensionUpdateState.UPDATE_AVAILABLE); + } finally { + await fs.rm(sourceDir, { recursive: true, force: true }); + } + }); + it('should convert a local Gemini archive before checking for updates', async () => { const tempDir = await fs.mkdtemp( path.join(os.tmpdir(), 'local-archive-update-test-'), diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index 29d411c07b6..5b0b272e7a0 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -325,19 +325,21 @@ export async function checkForExtensionUpdate( signal?.throwIfAborted(); await extractArchiveFile(installMetadata.source, tempDir, signal); signal?.throwIfAborted(); - const converted = await convertGeminiOrClaudeExtension( - tempDir, - installMetadata.pluginName, - installMetadata.networkPolicy, - signal, - installMetadata.pluginSourceKind, - ); - extensionDir = converted.extensionDir; - if (extensionDir !== tempDir) { - convertedDir = extensionDir; - } - signal?.throwIfAborted(); + extensionDir = tempDir; + } + const sourceBeforeConversion = extensionDir; + const converted = await convertGeminiOrClaudeExtension( + sourceBeforeConversion, + installMetadata.pluginName, + installMetadata.networkPolicy, + signal, + installMetadata.pluginSourceKind, + ); + extensionDir = converted.extensionDir; + if (extensionDir !== sourceBeforeConversion) { + convertedDir = extensionDir; } + signal?.throwIfAborted(); latestConfig = extensionManager.loadExtensionConfig({ extensionDir, }); From f8f86c931d0f57d6e780de0650728fa442ebdbba Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Sat, 8 Aug 2026 23:21:57 +0800 Subject: [PATCH 04/19] test(extensions): cover conversion safety fallbacks --- .../src/extension/extension-converter.test.ts | 107 ++++++++++++++++-- 1 file changed, 96 insertions(+), 11 deletions(-) diff --git a/packages/core/src/extension/extension-converter.test.ts b/packages/core/src/extension/extension-converter.test.ts index e6bc05b2a5d..81a2c9669ba 100644 --- a/packages/core/src/extension/extension-converter.test.ts +++ b/packages/core/src/extension/extension-converter.test.ts @@ -321,6 +321,75 @@ describe('convertGeminiOrClaudeExtension', () => { }, ); + it('ignores an escaping marketplace manifest when selecting dual-manifest root hooks', async () => { + fs.writeFileSync( + path.join(extensionDir, 'gemini-extension.json'), + JSON.stringify({ name: 'safe-root', version: '1.0.0' }), + ); + const manifestDir = path.join(extensionDir, '.claude-plugin'); + fs.mkdirSync(manifestDir, { recursive: true }); + fs.writeFileSync( + path.join(manifestDir, 'plugin.json'), + JSON.stringify({ + name: 'safe-root', + version: '1.0.0', + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: '${CLAUDE_PLUGIN_ROOT}/scripts/root-hook.sh', + }, + ], + }, + ], + }, + }), + ); + const outsideDir = fs.mkdtempSync( + path.join(os.tmpdir(), 'dual-marketplace-'), + ); + fs.writeFileSync( + path.join(outsideDir, 'marketplace.json'), + JSON.stringify({ + name: 'untrusted', + owner: { name: 'Untrusted' }, + plugins: [{ name: 'safe-root', source: './' }], + }), + ); + fs.symlinkSync( + path.join(outsideDir, 'marketplace.json'), + path.join(manifestDir, 'marketplace.json'), + ); + + try { + const converted = await convertGeminiOrClaudeExtension( + extensionDir, + 'safe-root', + ); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(converted.originSource).toBe('Gemini'); + expect(converted.requiresClaudeFileAdaptation).toBe(true); + expect( + ( + config.hooks?.['SessionStart']?.[0].hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe('${CLAUDE_PLUGIN_ROOT}/scripts/root-hook.sh'); + } finally { + fs.rmSync(outsideDir, { recursive: true, force: true }); + } + }); + it.each([ { format: 'Qwen', @@ -627,19 +696,35 @@ describe('convertGeminiOrClaudeExtension', () => { const manifestDir = path.join(extensionDir, '.claude-plugin'); fs.mkdirSync(manifestDir, { recursive: true }); fs.writeFileSync(path.join(manifestDir, 'plugin.json'), '{'); + const createTmpDir = ExtensionStorage.createTmpDir.bind(ExtensionStorage); + const tempDirs: string[] = []; + const createTmpDirSpy = vi + .spyOn(ExtensionStorage, 'createTmpDir') + .mockImplementation(async () => { + const tempDir = await createTmpDir(); + tempDirs.push(tempDir); + return tempDir; + }); - const converted = await convertGeminiOrClaudeExtension(extensionDir); - convertedDir = converted.extensionDir; - const config = JSON.parse( - fs.readFileSync( - path.join(converted.extensionDir, 'qwen-extension.json'), - 'utf-8', - ), - ) as ExtensionConfig; + try { + const converted = await convertGeminiOrClaudeExtension(extensionDir); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; - expect(converted.originSource).toBe('Gemini'); - expect(converted.requiresClaudeFileAdaptation).toBe(false); - expect(config.name).toBe('valid-gemini'); + expect(converted.originSource).toBe('Gemini'); + expect(converted.requiresClaudeFileAdaptation).toBe(false); + expect(config.name).toBe('valid-gemini'); + expect(tempDirs.filter((tempDir) => fs.existsSync(tempDir))).toEqual([ + converted.extensionDir, + ]); + } finally { + createTmpDirSpy.mockRestore(); + } }); it('removes the temporary Claude conversion after a successful merge', async () => { From 40652c498ab2ac7ff8995a3ca3641c7043fb6ed5 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Sun, 9 Aug 2026 22:25:04 +0800 Subject: [PATCH 05/19] fix(extensions): preserve marketplace plugin selection --- packages/core/src/extension/marketplace.ts | 2 +- .../core/src/extension/sourceRegistry.test.ts | 140 +++++++++++++++++- packages/core/src/extension/sourceRegistry.ts | 53 +++++-- 3 files changed, 180 insertions(+), 15 deletions(-) diff --git a/packages/core/src/extension/marketplace.ts b/packages/core/src/extension/marketplace.ts index 82141d8a78f..f8e2e53d4a3 100644 --- a/packages/core/src/extension/marketplace.ts +++ b/packages/core/src/extension/marketplace.ts @@ -41,7 +41,7 @@ export interface MarketplaceInstallResult { * - It's not part of a URL scheme (http://, https://, git@, sso://) * - It appears after the repo portion */ -function parseSourceAndPluginName(source: string): { +export function parseSourceAndPluginName(source: string): { repo: string; pluginName?: string; } { diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index 691618950a2..08f728990d3 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -14,8 +14,13 @@ import { discoverPlugins, type ExtensionSource, } from './sourceRegistry.js'; -import { loadMarketplaceConfigFromSource } from './marketplace.js'; +import { + loadMarketplaceConfigFromSource, + parseSourceAndPluginName, +} from './marketplace.js'; import type { ClaudeMarketplaceConfig } from './claude-converter.js'; +import { convertCompatibleExtension } from './extension-converter.js'; +import { QODER_PLUGIN_MANIFEST } from './qoder-converter.js'; vi.mock('./marketplace.js', async (importOriginal) => { const actual = await importOriginal(); @@ -308,6 +313,44 @@ describe('discoverPlugins', () => { url: 'https://github.com/someone/other-root-plugin', }, }, + { + name: 'selected-string-plugin', + version: '1.0.0', + source: 'someone/repo:selected', + }, + { + name: 'selected-url-plugin', + version: '1.0.0', + source: { + source: 'url', + url: 'https://github.com/someone/repo:selected', + }, + }, + { + name: 'port-root', + version: '1.0.0', + source: 'https://example.com:8443/root-plugin', + }, + { + name: 'port-selected', + version: '1.0.0', + source: 'https://example.com:8443/repo:selected', + }, + { + name: 'bare-root', + version: '1.0.0', + source: 'someone/direct-root', + }, + { + name: 'git-root', + version: '1.0.0', + source: 'git@github.com:someone/direct-root.git', + }, + { + name: 'sso-root', + version: '1.0.0', + source: 'sso://team/direct-root', + }, ]), ); @@ -340,6 +383,101 @@ describe('discoverPlugins', () => { expect( discovered.find((p) => p.name === 'url-github-root')!.pluginSourceKind, ).toBe('extension-root'); + expect( + discovered.find((p) => p.name === 'selected-string-plugin')! + .installSource, + ).toBe('someone/repo:selected'); + expect( + discovered.find((p) => p.name === 'selected-string-plugin')! + .pluginSourceKind, + ).toBe('marketplace-entry'); + expect( + discovered.find((p) => p.name === 'selected-url-plugin')!.installSource, + ).toBe('https://github.com/someone/repo:selected'); + expect( + discovered.find((p) => p.name === 'selected-url-plugin')! + .pluginSourceKind, + ).toBe('marketplace-entry'); + expect(discovered.find((p) => p.name === 'port-root')!.installSource).toBe( + 'https://example.com:8443/root-plugin', + ); + expect( + discovered.find((p) => p.name === 'port-root')!.pluginSourceKind, + ).toBe('extension-root'); + expect( + discovered.find((p) => p.name === 'port-selected')!.installSource, + ).toBe('https://example.com:8443/repo:selected'); + expect( + discovered.find((p) => p.name === 'port-selected')!.pluginSourceKind, + ).toBe('marketplace-entry'); + expect(discovered.find((p) => p.name === 'bare-root')).toMatchObject({ + installSource: 'someone/direct-root:bare-root', + pluginSourceKind: 'extension-root', + }); + expect(discovered.find((p) => p.name === 'git-root')).toMatchObject({ + installSource: 'git@github.com:someone/direct-root.git:git-root', + pluginSourceKind: 'extension-root', + }); + expect(discovered.find((p) => p.name === 'sso-root')).toMatchObject({ + installSource: 'sso://team/direct-root:sso-root', + pluginSourceKind: 'extension-root', + }); + }); + + it('keeps a structured GitHub Qoder plugin installable as a direct root', async () => { + vi.mocked(loadMarketplaceConfigFromSource).mockResolvedValue( + config('Remote', [ + { + name: 'qoder-alias', + version: '1.0.0', + source: { source: 'github', repo: 'someone/qoder-plugin' }, + }, + ]), + ); + const [plugin] = await discoverPlugins( + [{ name: 'Remote', source: 'https://x/m.json', type: 'http' }], + new Set(), + ); + const parsed = parseSourceAndPluginName(plugin.installSource); + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'qoder-discovery-')); + let convertedDir: string | undefined; + + try { + fs.mkdirSync(path.join(root, '.qoder-plugin'), { recursive: true }); + fs.writeFileSync( + path.join(root, QODER_PLUGIN_MANIFEST), + JSON.stringify({ name: 'qoder-root', version: '1.2.3' }), + 'utf-8', + ); + + const converted = await convertCompatibleExtension( + root, + parsed.pluginName, + undefined, + undefined, + plugin.pluginSourceKind, + ); + convertedDir = converted.extensionDir; + + expect(plugin).toMatchObject({ + installSource: 'someone/qoder-plugin:qoder-alias', + pluginSourceKind: 'extension-root', + }); + expect(converted.originSource).toBe('Qoder'); + expect( + JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ), + ).toMatchObject({ name: 'qoder-root', version: '1.2.3' }); + } finally { + if (convertedDir && convertedDir !== root) { + fs.rmSync(convertedDir, { recursive: true, force: true }); + } + fs.rmSync(root, { recursive: true, force: true }); + } }); it('rejects local-path sources from a remote (http) marketplace', async () => { diff --git a/packages/core/src/extension/sourceRegistry.ts b/packages/core/src/extension/sourceRegistry.ts index e89e110df73..7c71d349cbf 100644 --- a/packages/core/src/extension/sourceRegistry.ts +++ b/packages/core/src/extension/sourceRegistry.ts @@ -10,7 +10,10 @@ import { atomicWriteFileSync } from '../utils/atomicFileWrite.js'; import { stripAnsiAndControl } from '../utils/textUtils.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { redactUrlCredentials } from './redaction.js'; -import { loadMarketplaceConfigFromSource } from './marketplace.js'; +import { + loadMarketplaceConfigFromSource, + parseSourceAndPluginName, +} from './marketplace.js'; import { quarantineCorruptFile } from './corruptFile.js'; import type { ExtensionInstallMetadata, @@ -156,6 +159,35 @@ function isOwnerRepoShorthand(source: string): boolean { return /^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/.test(source); } +function classifyRemotePluginSource( + source: string, + fallbackPluginName?: string, +): { + installSource: string; + pluginSourceKind: ExtensionPluginSourceKind; +} { + const parsed = parseSourceAndPluginName(source); + if (parsed.pluginName) { + return { + installSource: source, + pluginSourceKind: 'marketplace-entry', + }; + } + if (fallbackPluginName) { + return { + installSource: `${source}:${fallbackPluginName}`, + // The appended name is only an install alias for a direct plugin root. + // It must not turn a plain repo/transport source into a marketplace + // selector: Claude and Qoder roots do not contain marketplace.json. + pluginSourceKind: 'extension-root', + }; + } + return { + installSource: source, + pluginSourceKind: 'extension-root', + }; +} + /** * Builds the install-source string fed to `parseInstallSource` for a discovered * plugin. For repo/local sources this is `:`, @@ -189,16 +221,14 @@ function resolveInstallSource( pluginSourceKind: 'extension-root', }; } - return { - installSource: src.includes(':') ? src : `${src}:${plugin.name}`, - pluginSourceKind: 'extension-root', - }; + const isDirectUrl = /^https?:\/\//i.test(src); + return classifyRemotePluginSource( + src, + isDirectUrl ? undefined : plugin.name, + ); } if (src && src.source === 'github') { - return { - installSource: `${src.repo}:${plugin.name}`, - pluginSourceKind: 'extension-root', - }; + return classifyRemotePluginSource(src.repo, plugin.name); } if (src && src.source === 'url') { // Same local-path guard as the string-source branch above: a remote @@ -218,10 +248,7 @@ function resolveInstallSource( pluginSourceKind: 'extension-root', }; } - return { - installSource: src.url, - pluginSourceKind: 'extension-root', - }; + return classifyRemotePluginSource(src.url); } return { installSource: plugin.name, From cea0397a1f63cd1f36140eb6e528cea6a72674cd Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:54:02 +0800 Subject: [PATCH 06/19] fix(review): keep repository context within file limit --- .qwen/review-context.json | 5 ++++- .../lib/manifest-repository-context.committed.test.ts | 8 ++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.qwen/review-context.json b/.qwen/review-context.json index f3c553bb593..cc36d315d12 100644 --- a/.qwen/review-context.json +++ b/.qwen/review-context.json @@ -21,7 +21,10 @@ }, { "paths": ["packages/core/src/skills/**"], - "relatedPaths": ["packages/core/src/skills/**"], + "relatedPaths": [ + "packages/core/src/skills/*.ts", + "packages/core/src/skills/bundled/*/SKILL.md" + ], "domains": ["core-skills"] }, { diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index 0b22dd7c11f..adeee4a60ec 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -47,7 +47,10 @@ const expectedManifest = { }, { paths: ['packages/core/src/skills/**'], - relatedPaths: ['packages/core/src/skills/**'], + relatedPaths: [ + 'packages/core/src/skills/*.ts', + 'packages/core/src/skills/bundled/*/SKILL.md', + ], domains: ['core-skills'], }, { @@ -95,7 +98,8 @@ const expectedManifest = { const relatedPathSentinels: Readonly> = { 'packages/core/src/config/**': 'packages/core/src/config/config.ts', - 'packages/core/src/skills/**': + 'packages/core/src/skills/*.ts': 'packages/core/src/skills/skill-manager.ts', + 'packages/core/src/skills/bundled/*/SKILL.md': 'packages/core/src/skills/bundled/review/SKILL.md', 'packages/web-shell/client/adapters/**': 'packages/web-shell/client/adapters/types.ts', From 5f6cbc17f0bcb3bf89fdb1769e0529a425c8fdaa Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 04:10:29 +0800 Subject: [PATCH 07/19] test(review): pin repository context headroom --- ...ifest-repository-context.committed.test.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index adeee4a60ec..57f5c50a0f1 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -47,6 +47,10 @@ const expectedManifest = { }, { paths: ['packages/core/src/skills/**'], + // Keep every bundled skill entrypoint, but deliberately leave nested + // implementations, tests, references, and scripts to the changed-file + // diff. Including bundled/*/** makes the all-rule union 129 files and + // violates the 128-item repository-context wire contract. relatedPaths: [ 'packages/core/src/skills/*.ts', 'packages/core/src/skills/bundled/*/SKILL.md', @@ -191,12 +195,39 @@ describe('committed review context manifest', () => { } }); + it('keeps nested bundled-skill material out of automatic related context', () => { + const context = provideForRepo([ + 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.js', + ]); + + expect(context).not.toBeNull(); + expect(context?.domains).toContain('core-skills'); + expect(context?.relatedPaths).toContain( + 'packages/core/src/skills/bundled/dataviz/SKILL.md', + ); + expect(context?.relatedPaths).not.toContain( + 'packages/core/src/skills/bundled/dataviz/references/palette.md', + ); + expect(context?.relatedPaths).not.toContain( + 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.js', + ); + expect(context?.relatedPaths).not.toContain( + 'packages/core/src/skills/bundled/loop/autonomous-loop.ts', + ); + }); + it('stays under the resolved-file bound when every rule co-matches', () => { const probes = expectedManifest.rules.flatMap((rule) => rule.paths.map(probeFor), ); const context = provideForRepo(probes); expect(context).not.toBeNull(); + + // At this revision the real provider resolves 115/128 files, leaving 13 + // slots. The manifest grammar has no negation globs, and hooks/** is the + // fastest-growing remaining group, so this bound is the deliberate alarm + // for future rebalancing rather than permission to truncate the result. + expect(context?.relatedPaths).toHaveLength(115); expect(context?.relatedPaths.length).toBeLessThanOrEqual(MAX_ARRAY_ITEMS); }); From d41df19f45e0db926f943649236eafc155f3eb19 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 05:28:40 +0800 Subject: [PATCH 08/19] test(review): make context bound probe robust --- ...ifest-repository-context.committed.test.ts | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index 57f5c50a0f1..54ccfb73da1 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -217,17 +217,32 @@ describe('committed review context manifest', () => { }); it('stays under the resolved-file bound when every rule co-matches', () => { - const probes = expectedManifest.rules.flatMap((rule) => - rule.paths.map(probeFor), - ); + const probes = expectedManifest.rules.map((rule) => { + const wildcardPattern = rule.paths.find((pattern) => + /[?*]/.test(pattern), + ); + expect( + wildcardPattern, + 'every rule needs a synthetic wildcard probe for the union test', + ).toBeDefined(); + const probe = probeFor(wildcardPattern as string); + expect( + existsSync(join(repoRoot, probe)), + 'union probes must not be real changed files excluded from relatedPaths', + ).toBe(false); + return probe; + }); const context = provideForRepo(probes); expect(context).not.toBeNull(); - // At this revision the real provider resolves 115/128 files, leaving 13 - // slots. The manifest grammar has no negation globs, and hooks/** is the - // fastest-growing remaining group, so this bound is the deliberate alarm - // for future rebalancing rather than permission to truncate the result. - expect(context?.relatedPaths).toHaveLength(115); + // The reviewed tree resolved 115/128 files, leaving 13 slots. Do not pin + // that exact count: the real provider intentionally scans the working tree, + // so harmless untracked files and normal source growth can change it. The + // synthetic, nonexistent changed paths above prevent changed-file exclusion + // from understating the union. The manifest grammar has no negation globs, + // and hooks/** is the fastest-growing remaining group, so this bound is the + // deliberate alarm for future rebalancing rather than permission to + // truncate the result. expect(context?.relatedPaths.length).toBeLessThanOrEqual(MAX_ARRAY_ITEMS); }); From 96171c4e6291b6557773cdb0e476bae50d080a5d Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 06:21:00 +0800 Subject: [PATCH 09/19] test(review): avoid changed-path exclusion false positive --- .../lib/manifest-repository-context.committed.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index 54ccfb73da1..eb270630517 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -196,9 +196,10 @@ describe('committed review context manifest', () => { }); it('keeps nested bundled-skill material out of automatic related context', () => { - const context = provideForRepo([ - 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.js', - ]); + const changedPath = + 'packages/core/src/skills/bundled/dataviz/synthetic-change.txt'; + expect(existsSync(join(repoRoot, changedPath))).toBe(false); + const context = provideForRepo([changedPath]); expect(context).not.toBeNull(); expect(context?.domains).toContain('core-skills'); From 76afb302cebe4bacc04ddccde0bb528299d522e5 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 07:35:04 +0800 Subject: [PATCH 10/19] test(review): harden manifest policy probes --- ...ifest-repository-context.committed.test.ts | 114 +++++++++++++----- 1 file changed, 87 insertions(+), 27 deletions(-) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index eb270630517..4ae18201745 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -5,7 +5,7 @@ */ import { execFileSync } from 'node:child_process'; -import { existsSync, readFileSync } from 'node:fs'; +import { existsSync, readFileSync, readdirSync } from 'node:fs'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; @@ -23,6 +23,7 @@ const repoRoot = resolve( ); const MANIFEST_RELATIVE_PATH = '.qwen/review-context.json'; +const BUNDLED_SKILLS_ROOT = 'packages/core/src/skills/bundled'; const expectedManifest = { version: 1, @@ -133,11 +134,44 @@ function provideForRepo(changedPaths: string[]) { }); } +function provideForPattern(pattern: string, changedPath: string) { + const manifest = { + version: 1, + label: 'Pattern probe', + rules: [{ paths: [pattern], domains: ['pattern-probe'] }], + }; + return manifestRepositoryContextProvider.provide({ + worktree: repoRoot, + changedPaths: [changedPath], + readIdentityFile: (relativePath) => + relativePath === MANIFEST_RELATIVE_PATH ? JSON.stringify(manifest) : null, + }); +} + function probeFor(pattern: string): string { - const wildcard = pattern.search(/[?*]/); - if (wildcard === -1) return pattern; - const slash = pattern.lastIndexOf('/', wildcard); - return `${pattern.slice(0, slash)}/probe.txt`; + return pattern + .split('/') + .map((segment) => + segment === '**' + ? '__qwen_review_probe__' + : segment.replaceAll('*', 'probe').replaceAll('?', 'q'), + ) + .join('/'); +} + +function listFilesRecursively(relativeDirectory: string): string[] { + const files: string[] = []; + for (const entry of readdirSync(join(repoRoot, relativeDirectory), { + withFileTypes: true, + })) { + const relativePath = `${relativeDirectory}/${entry.name}`; + if (entry.isDirectory()) { + files.push(...listFilesRecursively(relativePath)); + } else if (entry.isFile()) { + files.push(relativePath); + } + } + return files.sort(); } function inGitWorktree(): boolean { @@ -169,10 +203,15 @@ describe('committed review context manifest', () => { }); it('matches every paths pattern through the real provider', () => { - for (const rule of expectedManifest.rules) { - for (const pattern of rule.paths) { - expect(provideForRepo([probeFor(pattern)])).not.toBeNull(); - } + const patterns = [ + ...expectedManifest.rules.flatMap((rule) => rule.paths), + 'packages/core/src/synthetic/*.json', + ]; + for (const pattern of patterns) { + const probe = probeFor(pattern); + expect(provideForPattern(pattern, probe)?.domains).toEqual([ + 'pattern-probe', + ]); } }); @@ -203,35 +242,56 @@ describe('committed review context manifest', () => { expect(context).not.toBeNull(); expect(context?.domains).toContain('core-skills'); - expect(context?.relatedPaths).toContain( - 'packages/core/src/skills/bundled/dataviz/SKILL.md', - ); - expect(context?.relatedPaths).not.toContain( + + const nestedSentinels = [ 'packages/core/src/skills/bundled/dataviz/references/palette.md', - ); - expect(context?.relatedPaths).not.toContain( 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.js', - ); - expect(context?.relatedPaths).not.toContain( + 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.test.js', 'packages/core/src/skills/bundled/loop/autonomous-loop.ts', + 'packages/core/src/skills/bundled/loop/autonomous-loop.test.ts', + 'packages/core/src/skills/bundled/review/DESIGN.md', + ]; + for (const sentinel of nestedSentinels) { + expect(existsSync(join(repoRoot, sentinel))).toBe(true); + } + + const bundledFiles = listFilesRecursively(BUNDLED_SKILLS_ROOT); + const entrypoints = bundledFiles.filter((path) => + /^packages\/core\/src\/skills\/bundled\/[^/]+\/SKILL\.md$/.test(path), ); + const nestedFiles = bundledFiles.filter( + (path) => !entrypoints.includes(path), + ); + expect(entrypoints.length).toBeGreaterThan(0); + expect(nestedFiles).toEqual(expect.arrayContaining(nestedSentinels)); + for (const entrypoint of entrypoints) { + expect(context?.relatedPaths).toContain(entrypoint); + } + for (const nestedFile of nestedFiles) { + expect(context?.relatedPaths).not.toContain(nestedFile); + } }); it('stays under the resolved-file bound when every rule co-matches', () => { - const probes = expectedManifest.rules.map((rule) => { - const wildcardPattern = rule.paths.find((pattern) => + const probes = expectedManifest.rules.flatMap((rule) => { + const wildcardPatterns = rule.paths.filter((pattern) => /[?*]/.test(pattern), ); expect( - wildcardPattern, + wildcardPatterns.length, 'every rule needs a synthetic wildcard probe for the union test', - ).toBeDefined(); - const probe = probeFor(wildcardPattern as string); - expect( - existsSync(join(repoRoot, probe)), - 'union probes must not be real changed files excluded from relatedPaths', - ).toBe(false); - return probe; + ).toBeGreaterThan(0); + return wildcardPatterns.map((pattern) => { + const probe = probeFor(pattern); + expect( + existsSync(join(repoRoot, probe)), + 'union probes must not be real changed files excluded from relatedPaths', + ).toBe(false); + expect(provideForPattern(pattern, probe)?.domains).toEqual([ + 'pattern-probe', + ]); + return probe; + }); }); const context = provideForRepo(probes); expect(context).not.toBeNull(); From 13ef11e9270d28dc4201de05a61488810cc7b57b Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 08:29:36 +0800 Subject: [PATCH 11/19] test(review): cover all top-level skill sources --- .../manifest-repository-context.committed.test.ts | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index 4ae18201745..870b836bbd6 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -23,7 +23,8 @@ const repoRoot = resolve( ); const MANIFEST_RELATIVE_PATH = '.qwen/review-context.json'; -const BUNDLED_SKILLS_ROOT = 'packages/core/src/skills/bundled'; +const SKILLS_ROOT = 'packages/core/src/skills'; +const BUNDLED_SKILLS_ROOT = `${SKILLS_ROOT}/bundled`; const expectedManifest = { version: 1, @@ -255,6 +256,12 @@ describe('committed review context manifest', () => { expect(existsSync(join(repoRoot, sentinel))).toBe(true); } + const topLevelSources = readdirSync(join(repoRoot, SKILLS_ROOT), { + withFileTypes: true, + }) + .filter((entry) => entry.isFile() && entry.name.endsWith('.ts')) + .map((entry) => `${SKILLS_ROOT}/${entry.name}`) + .sort(); const bundledFiles = listFilesRecursively(BUNDLED_SKILLS_ROOT); const entrypoints = bundledFiles.filter((path) => /^packages\/core\/src\/skills\/bundled\/[^/]+\/SKILL\.md$/.test(path), @@ -262,8 +269,12 @@ describe('committed review context manifest', () => { const nestedFiles = bundledFiles.filter( (path) => !entrypoints.includes(path), ); + expect(topLevelSources.length).toBeGreaterThan(1); expect(entrypoints.length).toBeGreaterThan(0); expect(nestedFiles).toEqual(expect.arrayContaining(nestedSentinels)); + for (const source of topLevelSources) { + expect(context?.relatedPaths).toContain(source); + } for (const entrypoint of entrypoints) { expect(context?.relatedPaths).toContain(entrypoint); } From 975f74bf1e892ab6a48c12107afa0fd321ab3ad5 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 09:27:16 +0800 Subject: [PATCH 12/19] test(review): share repository file walker --- ...ifest-repository-context.committed.test.ts | 24 ++++++------------- .../review-digest-covers-only-bundled.test.ts | 18 ++------------ .../cli/src/commands/review/lib/test-utils.ts | 11 ++++++++- 3 files changed, 19 insertions(+), 34 deletions(-) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index 870b836bbd6..8359188c189 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -6,11 +6,12 @@ import { execFileSync } from 'node:child_process'; import { existsSync, readFileSync, readdirSync } from 'node:fs'; -import { dirname, join, resolve } from 'node:path'; +import { dirname, join, relative, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { manifestRepositoryContextProvider } from './manifest-repository-context.js'; import { MAX_ARRAY_ITEMS } from './repository-context.js'; +import { allFiles } from './test-utils.js'; const repoRoot = resolve( dirname(fileURLToPath(import.meta.url)), @@ -160,21 +161,6 @@ function probeFor(pattern: string): string { .join('/'); } -function listFilesRecursively(relativeDirectory: string): string[] { - const files: string[] = []; - for (const entry of readdirSync(join(repoRoot, relativeDirectory), { - withFileTypes: true, - })) { - const relativePath = `${relativeDirectory}/${entry.name}`; - if (entry.isDirectory()) { - files.push(...listFilesRecursively(relativePath)); - } else if (entry.isFile()) { - files.push(relativePath); - } - } - return files.sort(); -} - function inGitWorktree(): boolean { try { execFileSync('git', ['rev-parse', '--is-inside-work-tree'], { @@ -206,6 +192,8 @@ describe('committed review context manifest', () => { it('matches every paths pattern through the real provider', () => { const patterns = [ ...expectedManifest.rules.flatMap((rule) => rule.paths), + // No committed paths pattern uses a single-star segment; keep one + // synthetic probe so that branch of probeFor and the matcher stay covered. 'packages/core/src/synthetic/*.json', ]; for (const pattern of patterns) { @@ -262,7 +250,9 @@ describe('committed review context manifest', () => { .filter((entry) => entry.isFile() && entry.name.endsWith('.ts')) .map((entry) => `${SKILLS_ROOT}/${entry.name}`) .sort(); - const bundledFiles = listFilesRecursively(BUNDLED_SKILLS_ROOT); + const bundledFiles = [...allFiles(join(repoRoot, BUNDLED_SKILLS_ROOT))] + .map((path) => relative(repoRoot, path)) + .sort(); const entrypoints = bundledFiles.filter((path) => /^packages\/core\/src\/skills\/bundled\/[^/]+\/SKILL\.md$/.test(path), ); diff --git a/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts b/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts index f7cc30ed942..d9bfd519f5a 100644 --- a/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts +++ b/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts @@ -21,13 +21,7 @@ // sparse or partial clone fails this test without anything being wrong. import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { - mkdtempSync, - readFileSync, - readdirSync, - rmSync, - writeFileSync, -} from 'node:fs'; +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { basename, @@ -44,6 +38,7 @@ import { NOT_BUNDLED_FILE, NOT_BUNDLED_RE, } from './stale-bundle.js'; +import { allFiles } from './test-utils.js'; const repoRoot = resolve( import.meta.dirname, @@ -63,15 +58,6 @@ const reviewDir = join( 'review', ); -/** Every file under `dir`, tests and fixtures included. */ -function* allFiles(dir: string): Generator { - for (const e of readdirSync(dir, { withFileTypes: true })) { - const full = join(dir, e.name); - if (e.isDirectory()) yield* allFiles(full); - else if (e.isFile()) yield full; - } -} - /** * Whether a static `import`/`export … from` clause is a statement-level * type-only form esbuild erases wholesale: a leading `type` with a clause of diff --git a/packages/cli/src/commands/review/lib/test-utils.ts b/packages/cli/src/commands/review/lib/test-utils.ts index 6f96962d750..ca53ec9f883 100644 --- a/packages/cli/src/commands/review/lib/test-utils.ts +++ b/packages/cli/src/commands/review/lib/test-utils.ts @@ -4,12 +4,21 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { mkdirSync, writeFileSync } from 'node:fs'; +import { mkdirSync, readdirSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { tmpdir } from 'node:os'; import { PARSE_ARGS_REPORT } from './paths.js'; import { DIGEST_FILE } from './stale-bundle.js'; +/** Every regular directory entry under `dir`; symlinks are not followed. */ +export function* allFiles(dir: string): Generator { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + if (entry.isDirectory()) yield* allFiles(full); + else if (entry.isFile()) yield full; + } +} + /** Seed the report `parse-args` tees, so the effort fallback has something to read. */ export function seedParseArgs(dir: string, effort: unknown): void { mkdirSync(join(dir, dirname(PARSE_ARGS_REPORT)), { recursive: true }); From 0b67e7047c0846752f20243fc2689a0767562371 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:23:40 +0800 Subject: [PATCH 13/19] test(review): normalize walked repository paths --- ...anifest-repository-context.committed.test.ts | 17 ++++++++++++++--- .../cli/src/commands/review/lib/test-utils.ts | 12 +++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index 8359188c189..c7bd85fe103 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -6,12 +6,12 @@ import { execFileSync } from 'node:child_process'; import { existsSync, readFileSync, readdirSync } from 'node:fs'; -import { dirname, join, relative, resolve } from 'node:path'; +import { dirname, join, relative, resolve, win32 } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { manifestRepositoryContextProvider } from './manifest-repository-context.js'; import { MAX_ARRAY_ITEMS } from './repository-context.js'; -import { allFiles } from './test-utils.js'; +import { allFiles, toRepositoryPath } from './test-utils.js'; const repoRoot = resolve( dirname(fileURLToPath(import.meta.url)), @@ -251,7 +251,7 @@ describe('committed review context manifest', () => { .map((entry) => `${SKILLS_ROOT}/${entry.name}`) .sort(); const bundledFiles = [...allFiles(join(repoRoot, BUNDLED_SKILLS_ROOT))] - .map((path) => relative(repoRoot, path)) + .map((path) => toRepositoryPath(relative(repoRoot, path))) .sort(); const entrypoints = bundledFiles.filter((path) => /^packages\/core\/src\/skills\/bundled\/[^/]+\/SKILL\.md$/.test(path), @@ -273,6 +273,17 @@ describe('committed review context manifest', () => { } }); + it('normalizes Windows file-walk results before repository matching', () => { + const windowsPath = win32.relative( + 'C:\\repo', + 'C:\\repo\\packages\\core\\src\\skills\\bundled\\review\\SKILL.md', + ); + + expect(toRepositoryPath(windowsPath, win32.sep)).toBe( + 'packages/core/src/skills/bundled/review/SKILL.md', + ); + }); + it('stays under the resolved-file bound when every rule co-matches', () => { const probes = expectedManifest.rules.flatMap((rule) => { const wildcardPatterns = rule.paths.filter((pattern) => diff --git a/packages/cli/src/commands/review/lib/test-utils.ts b/packages/cli/src/commands/review/lib/test-utils.ts index ca53ec9f883..11883172ef2 100644 --- a/packages/cli/src/commands/review/lib/test-utils.ts +++ b/packages/cli/src/commands/review/lib/test-utils.ts @@ -5,7 +5,7 @@ */ import { mkdirSync, readdirSync, writeFileSync } from 'node:fs'; -import { dirname, join } from 'node:path'; +import { dirname, join, sep } from 'node:path'; import { tmpdir } from 'node:os'; import { PARSE_ARGS_REPORT } from './paths.js'; import { DIGEST_FILE } from './stale-bundle.js'; @@ -19,6 +19,16 @@ export function* allFiles(dir: string): Generator { } } +/** Normalize a host-native filesystem path for repository path matching. */ +export function toRepositoryPath( + filePath: string, + pathSeparator = sep, +): string { + return pathSeparator === '/' + ? filePath + : filePath.split(pathSeparator).join('/'); +} + /** Seed the report `parse-args` tees, so the effort fallback has something to read. */ export function seedParseArgs(dir: string, effort: unknown): void { mkdirSync(join(dir, dirname(PARSE_ARGS_REPORT)), { recursive: true }); From 3a898180555c0906981d0f80de7acaae25d56850 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:12:14 +0800 Subject: [PATCH 14/19] fix(extensions): streamline dual manifest conversion --- .../src/extension/claude-converter.test.ts | 48 ++++- .../core/src/extension/claude-converter.ts | 196 +++++++++++------- .../src/extension/extension-converter.test.ts | 66 +++++- .../core/src/extension/extension-converter.ts | 72 ++++--- .../core/src/extension/gemini-converter.ts | 38 ++-- packages/core/src/extension/github.test.ts | 32 ++- packages/core/src/extension/github.ts | 6 + .../core/src/extension/qoder-converter.ts | 4 + 8 files changed, 335 insertions(+), 127 deletions(-) diff --git a/packages/core/src/extension/claude-converter.test.ts b/packages/core/src/extension/claude-converter.test.ts index b6d372108ac..bd64939b4a7 100644 --- a/packages/core/src/extension/claude-converter.test.ts +++ b/packages/core/src/extension/claude-converter.test.ts @@ -986,7 +986,8 @@ describe('convertClaudePluginPackage', () => { expect( (result.config.hooks!['PostToolUse']![0].hooks![0] as { command: string }) .command, - ).toBe(`${pluginSourceDir}/scripts/post-install.sh`); + ).toBe(`${result.convertedDir}/scripts/post-install.sh`); + expect(fs.existsSync(result.convertedDir)).toBe(true); // Clean up converted directory fs.rmSync(result.convertedDir, { recursive: true, force: true }); @@ -1180,6 +1181,51 @@ describe('convertClaudePluginStandalone', () => { fs.rmSync(result.convertedDir, { recursive: true, force: true }); }); + it('stops a standalone conversion when its recursive copy is aborted', async () => { + const pluginDir = path.join(testDir, '.claude-plugin'); + fs.mkdirSync(pluginDir, { recursive: true }); + fs.writeFileSync( + path.join(pluginDir, 'plugin.json'), + JSON.stringify({ name: 'abort-copy', version: '1.0.0' }), + 'utf-8', + ); + const sourceDir = path.join(testDir, 'assets'); + fs.mkdirSync(sourceDir, { recursive: true }); + fs.writeFileSync(path.join(sourceDir, 'one.txt'), 'one', 'utf-8'); + fs.writeFileSync(path.join(sourceDir, 'two.txt'), 'two', 'utf-8'); + + const controller = new AbortController(); + const reason = new Error('conversion cancelled'); + const copyFile = fs.promises.copyFile.bind(fs.promises); + const copySpy = vi + .spyOn(fs.promises, 'copyFile') + .mockImplementation(async (...args) => { + await copyFile(...args); + controller.abort(reason); + }); + const createTmpDir = ExtensionStorage.createTmpDir.bind(ExtensionStorage); + const tempDirs: string[] = []; + const createTmpDirSpy = vi + .spyOn(ExtensionStorage, 'createTmpDir') + .mockImplementation(async () => { + const tempDir = await createTmpDir(); + tempDirs.push(tempDir); + return tempDir; + }); + + try { + await expect( + convertClaudePluginStandalone(testDir, false, controller.signal), + ).rejects.toBe(reason); + expect(copySpy).toHaveBeenCalledOnce(); + expect(tempDirs).toHaveLength(1); + expect(fs.existsSync(tempDirs[0])).toBe(false); + } finally { + copySpy.mockRestore(); + createTmpDirSpy.mockRestore(); + } + }); + it('throws when there is no .claude-plugin/plugin.json', async () => { await expect(convertClaudePluginStandalone(testDir)).rejects.toThrow( /Plugin configuration not found/, diff --git a/packages/core/src/extension/claude-converter.ts b/packages/core/src/extension/claude-converter.ts index f37c1ea8209..d13b750d852 100644 --- a/packages/core/src/extension/claude-converter.ts +++ b/packages/core/src/extension/claude-converter.ts @@ -249,7 +249,11 @@ export function convertClaudeAgentConfig( * Parses the YAML frontmatter, converts the configuration, and writes back. * @param agentsDir Directory containing agent markdown files */ -async function convertAgentFiles(agentsDir: string): Promise { +async function convertAgentFiles( + agentsDir: string, + signal?: AbortSignal, +): Promise { + signal?.throwIfAborted(); if (!fs.existsSync(agentsDir)) { return; } @@ -257,6 +261,7 @@ async function convertAgentFiles(agentsDir: string): Promise { const files = await fs.promises.readdir(agentsDir); for (const file of files) { + signal?.throwIfAborted(); if (!file.endsWith('.md')) continue; const filePath = path.join(agentsDir, file); @@ -322,6 +327,7 @@ ${systemPrompt} await fs.promises.writeFile(filePath, newContent, 'utf-8'); } catch (error) { + signal?.throwIfAborted(); debugLogger.warn( `[Claude Converter] Failed to convert agent file ${filePath}: ${error instanceof Error ? error.message : String(error)}`, ); @@ -547,6 +553,7 @@ export async function convertClaudePluginPackage( pluginSource, mergedConfig, preserveHookVariables, + signal, ); return { ...converted, externalContent }; } finally { @@ -596,6 +603,90 @@ export function resolvePluginRelativeFile( return resolved; } +type ClaudeHooks = { [K in HookEventName]?: HookDefinition[] }; + +function loadClaudePluginManifest(pluginSource: string): ClaudePluginConfig { + const pluginJsonPath = path.join( + pluginSource, + '.claude-plugin', + 'plugin.json', + ); + if (!fs.existsSync(pluginJsonPath)) { + throw new Error(`Plugin configuration not found at ${pluginJsonPath}`); + } + if (!realPathWithin(pluginJsonPath, pluginSource)) { + throw new Error( + `Plugin configuration at ${pluginJsonPath} resolves through a symlink outside the plugin`, + ); + } + + const parsedConfig: unknown = JSON.parse( + fs.readFileSync(pluginJsonPath, 'utf-8'), + ); + if ( + typeof parsedConfig !== 'object' || + parsedConfig === null || + Array.isArray(parsedConfig) + ) { + throw new Error( + `Invalid plugin configuration at ${pluginJsonPath}: expected a JSON object`, + ); + } + return parsedConfig as ClaudePluginConfig; +} + +function loadClaudeHooks( + pluginSource: string, + hooks: ClaudePluginConfig['hooks'], +): ClaudeHooks | undefined { + if (!hooks) return undefined; + if (typeof hooks !== 'string') return hooks; + + const hooksPath = resolvePluginRelativeFile(pluginSource, hooks); + if (!hooksPath || !fs.existsSync(hooksPath)) return undefined; + + try { + const parsedHooks: unknown = JSON.parse( + fs.readFileSync(hooksPath, 'utf-8'), + ); + if (typeof parsedHooks !== 'object' || parsedHooks === null) { + return undefined; + } + const hooksData = + 'hooks' in parsedHooks + ? (parsedHooks as { hooks?: unknown }).hooks + : parsedHooks; + return typeof hooksData === 'object' && hooksData !== null + ? (hooksData as ClaudeHooks) + : undefined; + } catch (error) { + debugLogger.warn( + `Failed to parse hooks file ${hooksPath}: ${error instanceof Error ? error.message : String(error)}`, + ); + return undefined; + } +} + +/** + * Loads only the Claude hook metadata needed when a sibling Gemini manifest + * owns the installed artifact. This avoids creating and deleting a second full + * converted copy of a dual-manifest repository merely to read its hooks. + */ +export function loadClaudePluginHooks( + pluginSource: string, + marketplacePlugin?: ClaudeMarketplacePluginConfig, + signal?: AbortSignal, +): ClaudeHooks | undefined { + signal?.throwIfAborted(); + const pluginConfig = loadClaudePluginManifest(pluginSource); + const mergedConfig = marketplacePlugin + ? mergeClaudeConfigs(marketplacePlugin, pluginConfig) + : pluginConfig; + const hooks = loadClaudeHooks(pluginSource, mergedConfig.hooks); + signal?.throwIfAborted(); + return hooks; +} + /** * Builds a converted Qwen extension directory from a resolved Claude plugin * source directory and its merged config. Shared by the marketplace-based @@ -606,7 +697,9 @@ export async function buildQwenExtensionFromPlugin( pluginSource: string, mergedConfig: ClaudePluginConfig, preserveHookVariables = false, + signal?: AbortSignal, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { + signal?.throwIfAborted(); // Resolve MCP servers from a JSON file path if needed. if (mergedConfig.mcpServers && typeof mergedConfig.mcpServers === 'string') { const mcpServersPath = resolvePluginRelativeFile( @@ -632,7 +725,7 @@ export async function buildQwenExtensionFromPlugin( const tmpDir = await ExtensionStorage.createTmpDir(); try { - await copyDirectory(pluginSource, tmpDir); + await copyDirectory(pluginSource, tmpDir, undefined, signal); // A standalone plugin's source is a full git clone; drop VCS metadata so // it isn't shipped into the installed extension. @@ -650,6 +743,7 @@ export async function buildQwenExtensionFromPlugin( ]; for (const { name, config } of resourceConfigs) { + signal?.throwIfAborted(); const folderPath = path.join(tmpDir, name); const sourceFolderPath = path.join(pluginSource, name); @@ -657,7 +751,7 @@ export async function buildQwenExtensionFromPlugin( if (fs.existsSync(folderPath)) { fs.rmSync(folderPath, { recursive: true, force: true }); } - await collectResources(config, pluginSource, folderPath); + await collectResources(config, pluginSource, folderPath, signal); } else if ( !fs.existsSync(sourceFolderPath) && fs.existsSync(folderPath) @@ -666,46 +760,25 @@ export async function buildQwenExtensionFromPlugin( } } - // Handle hooks from a file path if needed. - if (mergedConfig.hooks && typeof mergedConfig.hooks === 'string') { - const hooksPath = resolvePluginRelativeFile( - pluginSource, - mergedConfig.hooks, - ); - - if (hooksPath && fs.existsSync(hooksPath)) { - try { - const hooksContent = fs.readFileSync(hooksPath, 'utf-8'); - const parsedHooks = JSON.parse(hooksContent); - - let hooksData; - if (parsedHooks.hooks && typeof parsedHooks.hooks === 'object') { - hooksData = parsedHooks.hooks as { - [K in HookEventName]?: HookDefinition[]; - }; - } else { - hooksData = parsedHooks as { - [K in HookEventName]?: HookDefinition[]; - }; - } - - mergedConfig.hooks = preserveHookVariables - ? hooksData - : substituteHookVariables(hooksData, pluginSource); - } catch (error) { - debugLogger.warn( - `Failed to parse hooks file ${hooksPath}: ${error instanceof Error ? error.message : String(error)}`, - ); - } - } + // Resolve file-based hooks, then either preserve the variable for the + // installer or bind it to the converted directory that actually survives + // this call. Never bind a public result to a source staging directory that + // convertClaudePluginPackage deletes in its finally block. + const resolvedHooks = loadClaudeHooks(pluginSource, mergedConfig.hooks); + if (resolvedHooks) { + mergedConfig.hooks = preserveHookVariables + ? resolvedHooks + : substituteHookVariables(resolvedHooks, tmpDir); } const agentsDestDir = path.join(tmpDir, 'agents'); - await convertAgentFiles(agentsDestDir); + await convertAgentFiles(agentsDestDir, signal); + signal?.throwIfAborted(); const qwenConfig = convertClaudeToQwenConfig(mergedConfig); const qwenConfigPath = path.join(tmpDir, 'qwen-extension.json'); + signal?.throwIfAborted(); fs.writeFileSync( qwenConfigPath, JSON.stringify(qwenConfig, null, 2), @@ -737,40 +810,10 @@ export async function buildQwenExtensionFromPlugin( export async function convertClaudePluginStandalone( extensionDir: string, preserveHookVariables = false, + signal?: AbortSignal, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { - const pluginJsonPath = path.join( - extensionDir, - '.claude-plugin', - 'plugin.json', - ); - if (!fs.existsSync(pluginJsonPath)) { - throw new Error(`Plugin configuration not found at ${pluginJsonPath}`); - } - // The manifest may be a symlink in an untrusted clone; refuse to follow it - // outside the package (would read an arbitrary JSON-shaped host file). - if (!realPathWithin(pluginJsonPath, extensionDir)) { - throw new Error( - `Plugin configuration at ${pluginJsonPath} resolves through a symlink outside the plugin`, - ); - } - - const parsedConfig: unknown = JSON.parse( - fs.readFileSync(pluginJsonPath, 'utf-8'), - ); - // A plugin.json whose body is `null`, an array, or a scalar would otherwise - // throw an opaque `Cannot read properties of null` on the deref below. Fail - // with a clear message instead (the marketplace path tolerates this via - // `mergeClaudeConfigs`, so guard the standalone path to match). - if ( - typeof parsedConfig !== 'object' || - parsedConfig === null || - Array.isArray(parsedConfig) - ) { - throw new Error( - `Invalid plugin configuration at ${pluginJsonPath}: expected a JSON object`, - ); - } - const mergedConfig = parsedConfig as ClaudePluginConfig; + signal?.throwIfAborted(); + const mergedConfig = loadClaudePluginManifest(extensionDir); if (!mergedConfig.mcpServers) { const mcpJsonPath = path.join(extensionDir, '.mcp.json'); @@ -813,6 +856,7 @@ export async function convertClaudePluginStandalone( extensionDir, mergedConfig, preserveHookVariables, + signal, ); } @@ -829,18 +873,21 @@ async function collectResources( resourcePaths: string | string[], pluginRoot: string, destDir: string, + signal?: AbortSignal, ): Promise { + signal?.throwIfAborted(); const paths = Array.isArray(resourcePaths) ? resourcePaths : [resourcePaths]; // Create destination directory if (!fs.existsSync(destDir)) { - fs.mkdirSync(destDir, { recursive: true }); + await fs.promises.mkdir(destDir, { recursive: true }); } // Get the destination folder name (e.g., 'commands', 'skills', 'agents') const destFolderName = path.basename(destDir); for (const resourcePath of paths) { + signal?.throwIfAborted(); // Resource paths come from an untrusted manifest; confine them to the // plugin so a value like "/etc/ssh" or "../../secrets" can't be copied in. const resolvedPath = resolvePluginRelativeFile(pluginRoot, resourcePath); @@ -879,9 +926,11 @@ async function collectResources( cwd: resolvedPath, nodir: true, dot: false, + signal, }); for (const file of files) { + signal?.throwIfAborted(); const srcFile = path.join(resolvedPath, file); const destFile = path.join(finalDestDir, file); @@ -913,10 +962,10 @@ async function collectResources( // Ensure parent directory exists const destFileDir = path.dirname(destFile); if (!fs.existsSync(destFileDir)) { - fs.mkdirSync(destFileDir, { recursive: true }); + await fs.promises.mkdir(destFileDir, { recursive: true }); } - fs.copyFileSync(srcFile, destFile); + await fs.promises.copyFile(srcFile, destFile); } } else { // File entry (e.g. `agents: ["./agents/wiki-architect.md"]`). @@ -925,9 +974,10 @@ async function collectResources( // "already in the destination folder". const fileName = path.basename(resolvedPath); const destFile = path.join(destDir, fileName); - fs.copyFileSync(resolvedPath, destFile); + await fs.promises.copyFile(resolvedPath, destFile); } } + signal?.throwIfAborted(); } /** diff --git a/packages/core/src/extension/extension-converter.test.ts b/packages/core/src/extension/extension-converter.test.ts index a9a4e76de09..3a2406139da 100644 --- a/packages/core/src/extension/extension-converter.test.ts +++ b/packages/core/src/extension/extension-converter.test.ts @@ -103,6 +103,16 @@ describe('convertGeminiOrClaudeExtension', () => { pluginSourceKind: 'extension-root' as const, includeMarketplace: true, }, + { + installKind: 'matching direct root alias with a subplugin entry', + pluginName: 'ponytail-alias', + marketplacePluginName: 'ponytail-alias', + marketplaceSource: './plugins/ponytail-alias', + marketplaceHooks: './hooks/marketplace-hooks.json', + expectedHookPath: 'scripts/session-start.sh', + pluginSourceKind: 'extension-root' as const, + includeMarketplace: true, + }, { installKind: 'legacy direct root alias', pluginName: 'ponytail-alias', @@ -319,6 +329,9 @@ describe('convertGeminiOrClaudeExtension', () => { ? '${CLAUDE_PLUGIN_ROOT}/' + expectedHookPath : undefined, ); + expect(config.hooks?.['SessionStart']).toHaveLength( + expectedHookPath ? 1 : 0, + ); expect( fs.existsSync( path.join(converted.extensionDir, 'commands', 'ponytail.md'), @@ -352,6 +365,9 @@ describe('convertGeminiOrClaudeExtension', () => { command?: string; } )?.command; + expect(installedConfig.hooks?.['SessionStart']).toHaveLength( + expectedHookPath ? 1 : 0, + ); expect(installedCommand && path.normalize(installedCommand)).toBe( expectedHookPath ? path.join(installedDir, expectedHookPath) @@ -668,6 +684,45 @@ describe('convertGeminiOrClaudeExtension', () => { ); }); + it('converts a named subplugin for legacy metadata without a source kind', async () => { + const rootClaudeDir = path.join(extensionDir, '.claude-plugin'); + const pluginDir = path.join(extensionDir, 'plugins', 'legacy-child'); + fs.mkdirSync(rootClaudeDir, { recursive: true }); + fs.mkdirSync(path.join(pluginDir, '.claude-plugin'), { recursive: true }); + fs.writeFileSync( + path.join(rootClaudeDir, 'marketplace.json'), + JSON.stringify({ + name: 'legacy-marketplace', + owner: { name: 'Owner' }, + plugins: [ + { + name: 'legacy-child', + source: './plugins/legacy-child', + }, + ], + }), + ); + fs.writeFileSync( + path.join(pluginDir, '.claude-plugin', 'plugin.json'), + JSON.stringify({ name: 'legacy-child', version: '2.0.0' }), + ); + + const converted = await convertGeminiOrClaudeExtension( + extensionDir, + 'legacy-child', + ); + convertedDir = converted.extensionDir; + const config = JSON.parse( + fs.readFileSync( + path.join(converted.extensionDir, 'qwen-extension.json'), + 'utf-8', + ), + ) as ExtensionConfig; + + expect(converted.originSource).toBe('Claude'); + expect(config).toMatchObject({ name: 'legacy-child', version: '2.0.0' }); + }); + it('merges conventional Gemini hooks with Claude hooks by event', async () => { fs.writeFileSync( path.join(extensionDir, 'gemini-extension.json'), @@ -863,7 +918,7 @@ describe('convertGeminiOrClaudeExtension', () => { } }); - it('removes the temporary Claude conversion after a successful merge', async () => { + it('reads Claude hooks without creating a second converted copy', async () => { fs.writeFileSync( path.join(extensionDir, 'gemini-extension.json'), JSON.stringify({ name: 'cleanup-success', version: '1.0.0' }), @@ -887,16 +942,15 @@ describe('convertGeminiOrClaudeExtension', () => { try { const converted = await convertGeminiOrClaudeExtension(extensionDir); convertedDir = converted.extensionDir; - expect(tempDirs).toHaveLength(2); + expect(tempDirs).toHaveLength(1); expect(converted.extensionDir).toBe(tempDirs[0]); expect(fs.existsSync(tempDirs[0])).toBe(true); - expect(fs.existsSync(tempDirs[1])).toBe(false); } finally { createTmpDirSpy.mockRestore(); } }); - it('removes both conversions when a merge is aborted before writing', async () => { + it('removes the Gemini conversion when hook loading observes an abort', async () => { fs.writeFileSync( path.join(extensionDir, 'gemini-extension.json'), JSON.stringify({ name: 'cleanup-abort', version: '1.0.0' }), @@ -916,7 +970,7 @@ describe('convertGeminiOrClaudeExtension', () => { .mockImplementation(async () => { const tempDir = await createTmpDir(); tempDirs.push(tempDir); - if (tempDirs.length === 2) controller.abort(reason); + controller.abort(reason); return tempDir; }); @@ -929,7 +983,7 @@ describe('convertGeminiOrClaudeExtension', () => { controller.signal, ), ).rejects.toBe(reason); - expect(tempDirs).toHaveLength(2); + expect(tempDirs).toHaveLength(1); expect(tempDirs.every((tempDir) => !fs.existsSync(tempDir))).toBe(true); } finally { createTmpDirSpy.mockRestore(); diff --git a/packages/core/src/extension/extension-converter.ts b/packages/core/src/extension/extension-converter.ts index 4ff9d8a54eb..fee73e48105 100644 --- a/packages/core/src/extension/extension-converter.ts +++ b/packages/core/src/extension/extension-converter.ts @@ -15,6 +15,8 @@ import { import { convertClaudePluginPackage, convertClaudePluginStandalone, + loadClaudePluginHooks, + type ClaudeMarketplacePluginConfig, } from './claude-converter.js'; import { convertQoderPlugin, @@ -52,7 +54,11 @@ async function removeConvertedDirectory(directory: string): Promise { } type MarketplacePluginSelection = - | { location: 'root'; version?: string } + | { + location: 'root'; + version?: string; + plugin: ClaudeMarketplacePluginConfig; + } | { location: 'other' | 'missing-marketplace' }; function selectedMarketplacePlugin( @@ -102,13 +108,21 @@ function selectedMarketplacePlugin( // Claude marketplaces allow an entry without `source`; that entry refers // to the marketplace root itself. if (source === undefined || source === null) { - return { location: 'root', version }; + return { + location: 'root', + version, + plugin: selectedPlugin as unknown as ClaudeMarketplacePluginConfig, + }; } if (typeof source !== 'string') return { location: 'other' }; return path.resolve(path.join(extensionDir, source)) === path.resolve(extensionDir) - ? { location: 'root', version } + ? { + location: 'root', + version, + plugin: selectedPlugin as unknown as ClaudeMarketplacePluginConfig, + } : { location: 'other' }; } catch { return { location: 'other' }; @@ -254,23 +268,23 @@ export async function convertCompatibleExtension( } else if (hasQwenConfig) { newExtensionDir = extensionDir; } else if (isGeminiExtension && hasClaudePlugin) { - const geminiConversion = await convertGeminiExtensionPackage(extensionDir); - let claudeConversion: - | Awaited> - | Awaited> - | undefined; + const geminiConversion = await convertGeminiExtensionPackage( + extensionDir, + signal, + ); try { signal?.throwIfAborted(); + let claudeHooks: ExtensionHooks | undefined; + let claudeMetadataLoaded = false; try { - claudeConversion = rootMarketplacePluginName - ? await convertClaudePluginPackage( - extensionDir, - rootMarketplacePluginName, - networkPolicy, - signal, - true, - ) - : await convertClaudePluginStandalone(extensionDir, true); + claudeHooks = loadClaudePluginHooks( + extensionDir, + rootMarketplacePluginName && marketplaceSelection.location === 'root' + ? marketplaceSelection.plugin + : undefined, + signal, + ); + claudeMetadataLoaded = true; } catch (error) { signal?.throwIfAborted(); debugLogger.warn( @@ -280,7 +294,7 @@ export async function convertCompatibleExtension( ); } - if (!claudeConversion) { + if (!claudeMetadataLoaded) { newExtensionDir = geminiConversion.convertedDir; originSource = 'Gemini'; return { @@ -295,7 +309,6 @@ export async function convertCompatibleExtension( geminiConversion.convertedDir, ); const geminiHooks = geminiConversion.config.hooks ?? conventionalHooks; - const claudeHooks = claudeConversion.config.hooks; const mergedConfig = { ...geminiConversion.config, ...(rootMarketplacePluginName && @@ -313,22 +326,15 @@ export async function convertCompatibleExtension( ); newExtensionDir = geminiConversion.convertedDir; originSource = 'Gemini'; - externalContent = - 'externalContent' in claudeConversion - ? claudeConversion.externalContent - : false; requiresClaudeFileAdaptation = Boolean(conventionalHooks || claudeHooks); } catch (error) { await removeConvertedDirectory(geminiConversion.convertedDir); throw error; - } finally { - if (claudeConversion) { - await removeConvertedDirectory(claudeConversion.convertedDir); - } } } else if (isGeminiExtension) { - newExtensionDir = (await convertGeminiExtensionPackage(extensionDir)) - .convertedDir; + newExtensionDir = ( + await convertGeminiExtensionPackage(extensionDir, signal) + ).convertedDir; originSource = 'Gemini'; } else if (pluginName && !isExplicitExtensionRoot) { const converted = await convertClaudePluginPackage( @@ -347,11 +353,13 @@ export async function convertCompatibleExtension( originSource = 'Claude'; externalContent = converted.externalContent; } else if (fs.existsSync(path.join(extensionDir, QODER_PLUGIN_MANIFEST))) { - newExtensionDir = (await convertQoderPlugin(extensionDir)).convertedDir; + newExtensionDir = (await convertQoderPlugin(extensionDir, signal)) + .convertedDir; originSource = 'Qoder'; } else if (hasClaudePlugin) { - newExtensionDir = (await convertClaudePluginStandalone(extensionDir, true)) - .convertedDir; + newExtensionDir = ( + await convertClaudePluginStandalone(extensionDir, true, signal) + ).convertedDir; originSource = 'Claude'; } signal?.throwIfAborted(); diff --git a/packages/core/src/extension/gemini-converter.ts b/packages/core/src/extension/gemini-converter.ts index bc8b44e985f..d5ef2f61044 100644 --- a/packages/core/src/extension/gemini-converter.ts +++ b/packages/core/src/extension/gemini-converter.ts @@ -77,7 +77,9 @@ export function convertGeminiToQwenConfig( */ export async function convertGeminiExtensionPackage( extensionDir: string, + signal?: AbortSignal, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { + signal?.throwIfAborted(); const geminiConfig = convertGeminiToQwenConfig(extensionDir); // Create temporary directory for converted extension @@ -85,15 +87,16 @@ export async function convertGeminiExtensionPackage( try { // Step 1: Copy all files and directories to temporary directory - await copyDirectory(extensionDir, tmpDir); + await copyDirectory(extensionDir, tmpDir, undefined, signal); // Step 2: Convert TOML commands to Markdown in commands folder const commandsDir = path.join(tmpDir, 'commands'); if (fs.existsSync(commandsDir)) { - await convertCommandsDirectory(commandsDir); + await convertCommandsDirectory(commandsDir, signal); } // Step 3: Create qwen-extension.json with converted config + signal?.throwIfAborted(); const qwenConfigPath = path.join(tmpDir, 'qwen-extension.json'); fs.writeFileSync( qwenConfigPath, @@ -151,10 +154,12 @@ export async function copyDirectory( source: string, destination: string, confineRoot?: string, + signal?: AbortSignal, ): Promise { + signal?.throwIfAborted(); // Create destination directory if it doesn't exist if (!fs.existsSync(destination)) { - fs.mkdirSync(destination, { recursive: true }); + await fs.promises.mkdir(destination, { recursive: true }); } // Symlinks in an (untrusted) source are dereferenced and their *target* @@ -171,14 +176,15 @@ export async function copyDirectory( } } - const entries = fs.readdirSync(source, { withFileTypes: true }); + const entries = await fs.promises.readdir(source, { withFileTypes: true }); for (const entry of entries) { + signal?.throwIfAborted(); const sourcePath = path.join(source, entry.name); const destPath = path.join(destination, entry.name); if (entry.isDirectory()) { - await copyDirectory(sourcePath, destPath, root); + await copyDirectory(sourcePath, destPath, root, signal); } else if (entry.isSymbolicLink()) { // Resolve symlink and copy the target content, but only when the target // stays inside the package root. @@ -192,40 +198,47 @@ export async function copyDirectory( } const targetStat = fs.statSync(realPath); if (targetStat.isDirectory()) { - await copyDirectory(realPath, destPath, root); + await copyDirectory(realPath, destPath, root, signal); } else if (targetStat.isFile()) { - fs.copyFileSync(realPath, destPath); + await fs.promises.copyFile(realPath, destPath); } // Skip sockets, FIFOs, etc. } catch { + signal?.throwIfAborted(); // Skip broken symlinks } } else if (entry.isFile()) { - fs.copyFileSync(sourcePath, destPath); + await fs.promises.copyFile(sourcePath, destPath); } // Skip sockets, FIFOs, block devices, and character devices } + signal?.throwIfAborted(); } /** * Converts all TOML command files in a directory to Markdown format. * @param commandsDir Path to the commands directory */ -async function convertCommandsDirectory(commandsDir: string): Promise { +async function convertCommandsDirectory( + commandsDir: string, + signal?: AbortSignal, +): Promise { // Find all .toml files in the commands directory const tomlFiles = await glob('**/*.toml', { cwd: commandsDir, nodir: true, dot: false, + signal, }); // Convert each TOML file to Markdown for (const relativeFile of tomlFiles) { + signal?.throwIfAborted(); const tomlPath = path.join(commandsDir, relativeFile); try { // Read TOML file - const tomlContent = fs.readFileSync(tomlPath, 'utf-8'); + const tomlContent = await fs.promises.readFile(tomlPath, 'utf-8'); // Convert to Markdown const markdownContent = convertTomlToMarkdown(tomlContent); @@ -234,11 +247,12 @@ async function convertCommandsDirectory(commandsDir: string): Promise { const markdownPath = tomlPath.replace(/\.toml$/, '.md'); // Write Markdown file - fs.writeFileSync(markdownPath, markdownContent, 'utf-8'); + await fs.promises.writeFile(markdownPath, markdownContent, 'utf-8'); // Delete original TOML file - fs.unlinkSync(tomlPath); + await fs.promises.unlink(tomlPath); } catch (error) { + signal?.throwIfAborted(); debugLogger.warn( `Warning: Failed to convert command file ${relativeFile}: ${error instanceof Error ? error.message : String(error)}`, ); diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index 254dbc3f9e6..3927e64b9dc 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -32,6 +32,7 @@ import * as archiver from 'archiver'; import { ExtensionUpdateState, type Extension, + type ExtensionConfig, type ExtensionManager, } from './extensionManager.js'; import { getErrorMessage } from '../utils/errors.js'; @@ -990,6 +991,26 @@ describe('git extension helpers', () => { expect(result).toBe(ExtensionUpdateState.NOT_UPDATABLE); }); + it('does not refetch external content selected by a local marketplace', async () => { + const extension = createExtension({ + installMetadata: { + type: 'local', + source: '/local/marketplace', + pluginName: 'remote-child', + pluginSourceKind: 'marketplace-entry', + externalContent: true, + }, + }); + const mockManager = { + loadExtensionConfig: vi.fn(), + } as unknown as ExtensionManager; + + await expect( + checkForExtensionUpdate(extension, mockManager), + ).resolves.toBe(ExtensionUpdateState.NOT_UPDATABLE); + expect(mockManager.loadExtensionConfig).not.toHaveBeenCalled(); + }); + it('checks a selected marketplace entry when its local source is a directory', async () => { const sourceDir = await fs.mkdtemp( path.join(os.tmpdir(), 'local-marketplace-update-test-'), @@ -1022,15 +1043,19 @@ describe('git extension helpers', () => { path.join(pluginDir, '.claude-plugin', 'plugin.json'), JSON.stringify({ name: 'selected', version: '2.0.0' }), ); + const loadedNames: string[] = []; const mockManager = { loadExtensionConfig: vi.fn( - ({ extensionDir }: { extensionDir: string }) => - JSON.parse( + ({ extensionDir }: { extensionDir: string }) => { + const config = JSON.parse( fsSync.readFileSync( path.join(extensionDir, EXTENSIONS_CONFIG_FILENAME), 'utf-8', ), - ), + ) as ExtensionConfig; + loadedNames.push(config.name); + return config; + }, ), } as unknown as ExtensionManager; const installMetadata = { @@ -1051,6 +1076,7 @@ describe('git extension helpers', () => { expect(unchanged).toBe(ExtensionUpdateState.UP_TO_DATE); expect(outdated).toBe(ExtensionUpdateState.UPDATE_AVAILABLE); + expect(loadedNames).toEqual(['selected', 'selected']); } finally { await fs.rm(sourceDir, { recursive: true, force: true }); } diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index f9bd36f094e..6a2a1b63d5e 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -322,6 +322,12 @@ export async function checkForExtensionUpdate( if (installMetadata.source.startsWith('upload:')) { return ExtensionUpdateState.NOT_UPDATABLE; } + // A local marketplace can select content fetched from a separate remote + // repository. The outer local path does not pin or version that nested + // content, so an automatic update sweep must not refetch it implicitly. + if (installMetadata.externalContent === true) { + return ExtensionUpdateState.NOT_UPDATABLE; + } let latestConfig: ExtensionConfig | undefined; let tempDir: string | undefined; let convertedDir: string | undefined; diff --git a/packages/core/src/extension/qoder-converter.ts b/packages/core/src/extension/qoder-converter.ts index fb5db93bc39..09c97cd0192 100644 --- a/packages/core/src/extension/qoder-converter.ts +++ b/packages/core/src/extension/qoder-converter.ts @@ -196,12 +196,16 @@ function resolveContextFiles( export async function convertQoderPlugin( extensionDir: string, + signal?: AbortSignal, ): Promise<{ config: ExtensionConfig; convertedDir: string }> { + signal?.throwIfAborted(); const config = loadQoderConfig(extensionDir); config.mcpServers = resolveMcpServers(extensionDir, config.mcpServers); const converted = await buildQwenExtensionFromPlugin( extensionDir, config as ClaudePluginConfig, + false, + signal, ); const contextFileName = resolveContextFiles( converted.convertedDir, From cd2d4281a5c6bd6c60539a316fd56e875e6c5b3a Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:35:49 +0800 Subject: [PATCH 15/19] chore(review): drop temporary context CI backport --- .qwen/review-context.json | 5 +- ...ifest-repository-context.committed.test.ts | 153 ++---------------- .../review-digest-covers-only-bundled.test.ts | 18 ++- .../cli/src/commands/review/lib/test-utils.ts | 23 +-- 4 files changed, 34 insertions(+), 165 deletions(-) diff --git a/.qwen/review-context.json b/.qwen/review-context.json index a89b6d724ae..e5ea58b62fa 100644 --- a/.qwen/review-context.json +++ b/.qwen/review-context.json @@ -21,10 +21,7 @@ }, { "paths": ["packages/core/src/skills/**"], - "relatedPaths": [ - "packages/core/src/skills/*.ts", - "packages/core/src/skills/bundled/*/SKILL.md" - ], + "relatedPaths": ["packages/core/src/skills/**"], "domains": ["core-skills"] }, { diff --git a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts index cf63b58a035..59b8d65c267 100644 --- a/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts +++ b/packages/cli/src/commands/review/lib/manifest-repository-context.committed.test.ts @@ -5,13 +5,12 @@ */ import { execFileSync } from 'node:child_process'; -import { existsSync, readFileSync, readdirSync } from 'node:fs'; -import { dirname, join, relative, resolve, win32 } from 'node:path'; +import { existsSync, readFileSync } from 'node:fs'; +import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vitest'; import { manifestRepositoryContextProvider } from './manifest-repository-context.js'; import { MAX_ARRAY_ITEMS } from './repository-context.js'; -import { allFiles, toRepositoryPath } from './test-utils.js'; const repoRoot = resolve( dirname(fileURLToPath(import.meta.url)), @@ -24,8 +23,6 @@ const repoRoot = resolve( ); const MANIFEST_RELATIVE_PATH = '.qwen/review-context.json'; -const SKILLS_ROOT = 'packages/core/src/skills'; -const BUNDLED_SKILLS_ROOT = `${SKILLS_ROOT}/bundled`; const expectedManifest = { version: 1, @@ -50,14 +47,7 @@ const expectedManifest = { }, { paths: ['packages/core/src/skills/**'], - // Keep every bundled skill entrypoint, but deliberately leave nested - // implementations, tests, references, and scripts to the changed-file - // diff. Including bundled/*/** makes the all-rule union 129 files and - // violates the 128-item repository-context wire contract. - relatedPaths: [ - 'packages/core/src/skills/*.ts', - 'packages/core/src/skills/bundled/*/SKILL.md', - ], + relatedPaths: ['packages/core/src/skills/**'], domains: ['core-skills'], }, { @@ -102,8 +92,7 @@ const expectedManifest = { const relatedPathSentinels: Readonly> = { 'packages/core/src/config/**': 'packages/core/src/config/config.ts', - 'packages/core/src/skills/*.ts': 'packages/core/src/skills/skill-manager.ts', - 'packages/core/src/skills/bundled/*/SKILL.md': + 'packages/core/src/skills/**': 'packages/core/src/skills/bundled/review/SKILL.md', 'packages/web-shell/client/adapters/**': 'packages/web-shell/client/adapters/types.ts', @@ -127,29 +116,11 @@ function provideForRepo(changedPaths: string[]) { }); } -function provideForPattern(pattern: string, changedPath: string) { - const manifest = { - version: 1, - label: 'Pattern probe', - rules: [{ paths: [pattern], domains: ['pattern-probe'] }], - }; - return manifestRepositoryContextProvider.provide({ - worktree: repoRoot, - changedPaths: [changedPath], - readIdentityFile: (relativePath) => - relativePath === MANIFEST_RELATIVE_PATH ? JSON.stringify(manifest) : null, - }); -} - function probeFor(pattern: string): string { - return pattern - .split('/') - .map((segment) => - segment === '**' - ? '__qwen_review_probe__' - : segment.replaceAll('*', 'probe').replaceAll('?', 'q'), - ) - .join('/'); + const wildcard = pattern.search(/[?*]/); + if (wildcard === -1) return pattern; + const slash = pattern.lastIndexOf('/', wildcard); + return `${pattern.slice(0, slash)}/probe.txt`; } function inGitWorktree(): boolean { @@ -181,17 +152,10 @@ describe('committed review context manifest', () => { }); it('matches every paths pattern through the real provider', () => { - const patterns = [ - ...expectedManifest.rules.flatMap((rule) => rule.paths), - // No committed paths pattern uses a single-star segment; keep one - // synthetic probe so that branch of probeFor and the matcher stay covered. - 'packages/core/src/synthetic/*.json', - ]; - for (const pattern of patterns) { - const probe = probeFor(pattern); - expect(provideForPattern(pattern, probe)?.domains).toEqual([ - 'pattern-probe', - ]); + for (const rule of expectedManifest.rules) { + for (const pattern of rule.paths) { + expect(provideForRepo([probeFor(pattern)])).not.toBeNull(); + } } }); @@ -214,99 +178,12 @@ describe('committed review context manifest', () => { } }); - it('keeps nested bundled-skill material out of automatic related context', () => { - const changedPath = - 'packages/core/src/skills/bundled/dataviz/synthetic-change.txt'; - expect(existsSync(join(repoRoot, changedPath))).toBe(false); - const context = provideForRepo([changedPath]); - - expect(context).not.toBeNull(); - expect(context?.domains).toContain('core-skills'); - - const nestedSentinels = [ - 'packages/core/src/skills/bundled/dataviz/references/palette.md', - 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.js', - 'packages/core/src/skills/bundled/dataviz/scripts/validate_palette.test.js', - 'packages/core/src/skills/bundled/loop/autonomous-loop.ts', - 'packages/core/src/skills/bundled/loop/autonomous-loop.test.ts', - 'packages/core/src/skills/bundled/review/DESIGN.md', - ]; - for (const sentinel of nestedSentinels) { - expect(existsSync(join(repoRoot, sentinel))).toBe(true); - } - - const topLevelSources = readdirSync(join(repoRoot, SKILLS_ROOT), { - withFileTypes: true, - }) - .filter((entry) => entry.isFile() && entry.name.endsWith('.ts')) - .map((entry) => `${SKILLS_ROOT}/${entry.name}`) - .sort(); - const bundledFiles = [...allFiles(join(repoRoot, BUNDLED_SKILLS_ROOT))] - .map((path) => toRepositoryPath(relative(repoRoot, path))) - .sort(); - const entrypoints = bundledFiles.filter((path) => - /^packages\/core\/src\/skills\/bundled\/[^/]+\/SKILL\.md$/.test(path), - ); - const nestedFiles = bundledFiles.filter( - (path) => !entrypoints.includes(path), - ); - expect(topLevelSources.length).toBeGreaterThan(1); - expect(entrypoints.length).toBeGreaterThan(0); - expect(nestedFiles).toEqual(expect.arrayContaining(nestedSentinels)); - for (const source of topLevelSources) { - expect(context?.relatedPaths).toContain(source); - } - for (const entrypoint of entrypoints) { - expect(context?.relatedPaths).toContain(entrypoint); - } - for (const nestedFile of nestedFiles) { - expect(context?.relatedPaths).not.toContain(nestedFile); - } - }); - - it('normalizes Windows file-walk results before repository matching', () => { - const windowsPath = win32.relative( - 'C:\\repo', - 'C:\\repo\\packages\\core\\src\\skills\\bundled\\review\\SKILL.md', - ); - - expect(toRepositoryPath(windowsPath, win32.sep)).toBe( - 'packages/core/src/skills/bundled/review/SKILL.md', - ); - }); - it('stays under the resolved-file bound when every rule co-matches', () => { - const probes = expectedManifest.rules.flatMap((rule) => { - const wildcardPatterns = rule.paths.filter((pattern) => - /[?*]/.test(pattern), - ); - expect( - wildcardPatterns.length, - 'every rule needs a synthetic wildcard probe for the union test', - ).toBeGreaterThan(0); - return wildcardPatterns.map((pattern) => { - const probe = probeFor(pattern); - expect( - existsSync(join(repoRoot, probe)), - 'union probes must not be real changed files excluded from relatedPaths', - ).toBe(false); - expect(provideForPattern(pattern, probe)?.domains).toEqual([ - 'pattern-probe', - ]); - return probe; - }); - }); + const probes = expectedManifest.rules.flatMap((rule) => + rule.paths.map(probeFor), + ); const context = provideForRepo(probes); expect(context).not.toBeNull(); - - // The reviewed tree resolved 115/128 files, leaving 13 slots. Do not pin - // that exact count: the real provider intentionally scans the working tree, - // so harmless untracked files and normal source growth can change it. The - // synthetic, nonexistent changed paths above prevent changed-file exclusion - // from understating the union. The manifest grammar has no negation globs, - // and hooks/** is the fastest-growing remaining group, so this bound is the - // deliberate alarm for future rebalancing rather than permission to - // truncate the result. expect(context?.relatedPaths.length).toBeLessThanOrEqual(MAX_ARRAY_ITEMS); }); diff --git a/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts b/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts index d9bfd519f5a..f7cc30ed942 100644 --- a/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts +++ b/packages/cli/src/commands/review/lib/review-digest-covers-only-bundled.test.ts @@ -21,7 +21,13 @@ // sparse or partial clone fails this test without anything being wrong. import { afterEach, beforeEach, describe, expect, it } from 'vitest'; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { + mkdtempSync, + readFileSync, + readdirSync, + rmSync, + writeFileSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { basename, @@ -38,7 +44,6 @@ import { NOT_BUNDLED_FILE, NOT_BUNDLED_RE, } from './stale-bundle.js'; -import { allFiles } from './test-utils.js'; const repoRoot = resolve( import.meta.dirname, @@ -58,6 +63,15 @@ const reviewDir = join( 'review', ); +/** Every file under `dir`, tests and fixtures included. */ +function* allFiles(dir: string): Generator { + for (const e of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, e.name); + if (e.isDirectory()) yield* allFiles(full); + else if (e.isFile()) yield full; + } +} + /** * Whether a static `import`/`export … from` clause is a statement-level * type-only form esbuild erases wholesale: a leading `type` with a clause of diff --git a/packages/cli/src/commands/review/lib/test-utils.ts b/packages/cli/src/commands/review/lib/test-utils.ts index 11883172ef2..6f96962d750 100644 --- a/packages/cli/src/commands/review/lib/test-utils.ts +++ b/packages/cli/src/commands/review/lib/test-utils.ts @@ -4,31 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { mkdirSync, readdirSync, writeFileSync } from 'node:fs'; -import { dirname, join, sep } from 'node:path'; +import { mkdirSync, writeFileSync } from 'node:fs'; +import { dirname, join } from 'node:path'; import { tmpdir } from 'node:os'; import { PARSE_ARGS_REPORT } from './paths.js'; import { DIGEST_FILE } from './stale-bundle.js'; -/** Every regular directory entry under `dir`; symlinks are not followed. */ -export function* allFiles(dir: string): Generator { - for (const entry of readdirSync(dir, { withFileTypes: true })) { - const full = join(dir, entry.name); - if (entry.isDirectory()) yield* allFiles(full); - else if (entry.isFile()) yield full; - } -} - -/** Normalize a host-native filesystem path for repository path matching. */ -export function toRepositoryPath( - filePath: string, - pathSeparator = sep, -): string { - return pathSeparator === '/' - ? filePath - : filePath.split(pathSeparator).join('/'); -} - /** Seed the report `parse-args` tees, so the effort fallback has something to read. */ export function seedParseArgs(dir: string, effort: unknown): void { mkdirSync(join(dir, dirname(PARSE_ARGS_REPORT)), { recursive: true }); From be195450a24d1a87d7dcf2d5b84cad6e8cb494cb Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 16:58:31 +0800 Subject: [PATCH 16/19] fix(extensions): address final review blockers --- packages/core/src/extension/github.test.ts | 21 +++++++++++ packages/core/src/extension/github.ts | 3 ++ .../src/extension/qoder-converter.test.ts | 29 +++++++++++++++ .../core/src/extension/qoder-converter.ts | 2 +- .../core/src/extension/sourceRegistry.test.ts | 37 +++++++++++++++++++ packages/core/src/extension/sourceRegistry.ts | 11 +++--- 6 files changed, 96 insertions(+), 7 deletions(-) diff --git a/packages/core/src/extension/github.test.ts b/packages/core/src/extension/github.test.ts index 3927e64b9dc..1df87699fe4 100644 --- a/packages/core/src/extension/github.test.ts +++ b/packages/core/src/extension/github.test.ts @@ -1372,6 +1372,27 @@ describe('git extension helpers', () => { } }); + it('does not refetch external content selected by an archive URL marketplace', async () => { + const extension = createExtension({ + installMetadata: { + type: 'archive-url', + source: 'https://example.com/catalog.zip', + pluginName: 'remote-child', + pluginSourceKind: 'marketplace-entry', + externalContent: true, + }, + }); + const mockManager = { + loadExtensionConfig: vi.fn(), + } as unknown as ExtensionManager; + + await expect( + checkForExtensionUpdate(extension, mockManager), + ).resolves.toBe(ExtensionUpdateState.NOT_UPDATABLE); + expect(mockHttpsGet).not.toHaveBeenCalled(); + expect(mockManager.loadExtensionConfig).not.toHaveBeenCalled(); + }); + it('should convert an archive URL Gemini archive before checking for updates', async () => { const tempDir = await fs.mkdtemp( path.join(os.tmpdir(), 'archive-url-update-test-'), diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index 6a2a1b63d5e..577010b47a8 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -394,6 +394,9 @@ export async function checkForExtensionUpdate( return checkNpmUpdate(installMetadata, signal); } if (installMetadata?.type === 'archive-url') { + if (installMetadata.externalContent === true) { + return ExtensionUpdateState.NOT_UPDATABLE; + } let tempDir: string | undefined; let convertedDir: string | undefined; try { diff --git a/packages/core/src/extension/qoder-converter.test.ts b/packages/core/src/extension/qoder-converter.test.ts index d6ca4faec9d..a382e643d7f 100644 --- a/packages/core/src/extension/qoder-converter.test.ts +++ b/packages/core/src/extension/qoder-converter.test.ts @@ -113,6 +113,35 @@ describe('convertQoderPlugin', () => { fs.rmSync(result.convertedDir, { recursive: true, force: true }); }); + it('preserves hook root variables for installation-time hydration', async () => { + writeManifest({ + name: 'sample-qoder-plugin', + hooks: { + SessionStart: [ + { + hooks: [ + { + type: 'command', + command: '${CLAUDE_PLUGIN_ROOT}/scripts/start.sh', + }, + ], + }, + ], + }, + }); + + const result = await convertQoderPlugin(root); + + expect( + ( + result.config.hooks?.['SessionStart']?.[0]?.hooks?.[0] as { + command?: string; + } + )?.command, + ).toBe('${CLAUDE_PLUGIN_ROOT}/scripts/start.sh'); + fs.rmSync(result.convertedDir, { recursive: true, force: true }); + }); + it('defaults the version and reports Qoder as the origin', async () => { writeManifest({ name: 'sample-qoder-plugin' }); fs.writeFileSync( diff --git a/packages/core/src/extension/qoder-converter.ts b/packages/core/src/extension/qoder-converter.ts index 09c97cd0192..6866f45064d 100644 --- a/packages/core/src/extension/qoder-converter.ts +++ b/packages/core/src/extension/qoder-converter.ts @@ -204,7 +204,7 @@ export async function convertQoderPlugin( const converted = await buildQwenExtensionFromPlugin( extensionDir, config as ClaudePluginConfig, - false, + true, signal, ); const contextFileName = resolveContextFiles( diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index 1bc80d97a75..8841d1e8412 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -440,6 +440,43 @@ describe('discoverPlugins', () => { }); }); + it.each([ + ['a structured GitHub source without repo', { source: 'github' }], + ['a structured URL source without url', { source: 'url' }], + ])('keeps valid siblings when %s is malformed', async (_, source) => { + vi.mocked(loadMarketplaceConfigFromSource).mockResolvedValue( + config('Remote', [ + { + name: 'malformed', + version: '1.0.0', + source: source as never, + }, + { + name: 'valid', + version: '1.0.0', + source: 'https://example.com/valid.tgz', + }, + ]), + ); + + const discovered = await discoverPlugins( + [{ name: 'Remote', source: 'https://x/m.json', type: 'http' }], + new Set(), + ); + + expect(discovered).toHaveLength(2); + expect( + discovered.find((plugin) => plugin.name === 'malformed'), + ).toMatchObject({ + installSource: '', + pluginSourceKind: 'extension-root', + }); + expect(discovered.find((plugin) => plugin.name === 'valid')).toMatchObject({ + installSource: 'https://example.com/valid.tgz', + pluginSourceKind: 'extension-root', + }); + }); + it('keeps a structured GitHub Qoder plugin installable as a direct root', async () => { vi.mocked(loadMarketplaceConfigFromSource).mockResolvedValue( config('Remote', [ diff --git a/packages/core/src/extension/sourceRegistry.ts b/packages/core/src/extension/sourceRegistry.ts index e88e5d11ba9..b0b8dec908a 100644 --- a/packages/core/src/extension/sourceRegistry.ts +++ b/packages/core/src/extension/sourceRegistry.ts @@ -226,18 +226,17 @@ function resolveInstallSource( isDirectUrl ? undefined : plugin.name, ); } - if (src && src.source === 'github') { + if (src && src.source === 'github' && typeof src.repo === 'string') { return classifyRemotePluginSource(src.repo, plugin.name); } - if (src && src.source === 'url') { + if (src && src.source === 'url' && typeof src.url === 'string') { // Same local-path guard as the string-source branch above: a remote // marketplace must not be able to redirect the installer at a local // filesystem path via the structured `{ source: 'url' }` form either. if ( - typeof src.url === 'string' && - (path.isAbsolute(src.url) || - src.url.startsWith('.') || - src.url.startsWith('~')) + path.isAbsolute(src.url) || + src.url.startsWith('.') || + src.url.startsWith('~') ) { debugLogger.warn( `Ignoring local path source "${src.url}" from remote marketplace "${marketplace.source}".`, From 0b4b30c2a518fc16ade6246bb046780434f1844d Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Thu, 13 Aug 2026 21:21:52 +0800 Subject: [PATCH 17/19] fix(extensions): preserve numeric direct-root aliases --- .../core/src/extension/marketplace.test.ts | 31 +++++++++++++++++++ packages/core/src/extension/marketplace.ts | 5 ++- .../core/src/extension/sourceRegistry.test.ts | 17 ++++++++++ 3 files changed, 52 insertions(+), 1 deletion(-) diff --git a/packages/core/src/extension/marketplace.test.ts b/packages/core/src/extension/marketplace.test.ts index 5e17bb58883..b1507ca4627 100644 --- a/packages/core/src/extension/marketplace.test.ts +++ b/packages/core/src/extension/marketplace.test.ts @@ -168,6 +168,15 @@ describe('parseInstallSource', () => { expect(result.pluginName).toBeUndefined(); }); + it('should not treat a trailing HTTPS port as a plugin name', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + + const result = await parseInstallSource('https://example.com:8080'); + + expect(result.source).toBe('https://example.com:8080'); + expect(result.pluginName).toBeUndefined(); + }); + it('should parse an uppercase HTTPS URL scheme as a git source', async () => { // Mock stat to fail (not a local path) vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); @@ -259,6 +268,28 @@ describe('parseInstallSource', () => { expect(result.pluginName).toBe('2048-game'); expect(result.pluginSourceKind).toBe('extension-root'); }); + + it.each([ + [ + 'git@github.com:owner/repo.git:2048', + 'git@github.com:owner/repo.git', + '2048', + ], + ['sso://team/repo:2049', 'sso://team/repo', '2049'], + ])( + 'parses an all-digit direct-root alias in %s', + async (installSource, source, pluginName) => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + + const result = await parseInstallSource(installSource, { + pluginSourceKind: 'extension-root', + }); + + expect(result.source).toBe(source); + expect(result.pluginName).toBe(pluginName); + expect(result.pluginSourceKind).toBe('extension-root'); + }, + ); }); describe('local path parsing', () => { diff --git a/packages/core/src/extension/marketplace.ts b/packages/core/src/extension/marketplace.ts index a922b452804..71610061267 100644 --- a/packages/core/src/extension/marketplace.ts +++ b/packages/core/src/extension/marketplace.ts @@ -68,7 +68,10 @@ export function parseSourceAndPluginName(source: string): { if ( potentialPluginName && !potentialPluginName.includes('/') && - !/^\d+$/.test(potentialPluginName) + !( + (scheme === 'http://' || scheme === 'https://') && + /^\d+$/.test(potentialPluginName) + ) ) { repoEndIndex = scheme.length + lastColonIndex; hasPluginName = true; diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index 8841d1e8412..2464e168038 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -351,6 +351,11 @@ describe('discoverPlugins', () => { version: '1.0.0', source: 'sso://team/direct-root', }, + { + name: '2048', + version: '1.0.0', + source: 'git@github.com:someone/numeric-root.git', + }, ]), ); @@ -438,6 +443,18 @@ describe('discoverPlugins', () => { repo: 'sso://team/direct-root', pluginName: '2048-sso', }); + expect(discovered.find((p) => p.name === '2048')).toMatchObject({ + installSource: 'git@github.com:someone/numeric-root.git:2048', + pluginSourceKind: 'extension-root', + }); + expect( + parseSourceAndPluginName( + discovered.find((p) => p.name === '2048')!.installSource, + ), + ).toEqual({ + repo: 'git@github.com:someone/numeric-root.git', + pluginName: '2048', + }); }); it.each([ From 47ca61cb531e4c87488987f41f4eaa290d3558c0 Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:18:41 +0800 Subject: [PATCH 18/19] fix(extensions): harden remote plugin source handling --- .../src/extension/extension-converter.test.ts | 27 +++++++++ .../core/src/extension/extension-converter.ts | 4 +- .../core/src/extension/sourceRegistry.test.ts | 31 ++++++++-- packages/core/src/extension/sourceRegistry.ts | 59 ++++++++++++++----- 4 files changed, 98 insertions(+), 23 deletions(-) diff --git a/packages/core/src/extension/extension-converter.test.ts b/packages/core/src/extension/extension-converter.test.ts index 3a2406139da..1063d1b43c9 100644 --- a/packages/core/src/extension/extension-converter.test.ts +++ b/packages/core/src/extension/extension-converter.test.ts @@ -1046,6 +1046,33 @@ describe('Agent Plugins extension conversion', () => { }); }); + it('does not parse a stray malformed Gemini manifest before selecting an Agent Plugin', async () => { + fs.writeFileSync( + path.join(pluginRoot, 'plugin.json'), + JSON.stringify({ + $schema: AGENT_PLUGIN_SCHEMA, + name: 'portable-plugin', + }), + ); + fs.writeFileSync( + path.join(pluginRoot, 'gemini-extension.json'), + '{ not valid json', + ); + + await expect( + convertGeminiOrClaudeExtension( + pluginRoot, + 'catalog-alias', + undefined, + undefined, + 'extension-root', + ), + ).resolves.toMatchObject({ + extensionDir: pluginRoot, + originSource: 'AgentPlugins', + }); + }); + it('gives an unsupported Agent Plugins schema priority over Qwen format', async () => { fs.writeFileSync( path.join(pluginRoot, 'plugin.json'), diff --git a/packages/core/src/extension/extension-converter.ts b/packages/core/src/extension/extension-converter.ts index fee73e48105..e997432042d 100644 --- a/packages/core/src/extension/extension-converter.ts +++ b/packages/core/src/extension/extension-converter.ts @@ -215,7 +215,9 @@ export async function convertCompatibleExtension( ); const hasQwenConfig = fs.existsSync(configFilePath); const isGeminiExtension = - !hasQwenConfig && isGeminiExtensionConfig(extensionDir); + agentPluginStatus === 'unrelated' && + !hasQwenConfig && + isGeminiExtensionConfig(extensionDir); const hasClaudePlugin = fs.existsSync( path.join(extensionDir, SUPPORTED_EXTENSION_MANIFESTS[3]), ); diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index 2464e168038..2e97bb5984e 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -365,7 +365,7 @@ describe('discoverPlugins', () => { ); expect(discovered.find((p) => p.name === 'gh-plugin')!.installSource).toBe( - 'someone/repo:gh-plugin', + 'https://github.com/someone/repo:gh-plugin', ); expect( discovered.find((p) => p.name === 'gh-plugin')!.pluginSourceKind, @@ -391,7 +391,7 @@ describe('discoverPlugins', () => { expect( discovered.find((p) => p.name === 'selected-string-plugin')! .installSource, - ).toBe('someone/repo:selected'); + ).toBe('https://github.com/someone/repo:selected'); expect( discovered.find((p) => p.name === 'selected-string-plugin')! .pluginSourceKind, @@ -416,7 +416,7 @@ describe('discoverPlugins', () => { discovered.find((p) => p.name === 'port-selected')!.pluginSourceKind, ).toBe('marketplace-entry'); expect(discovered.find((p) => p.name === 'bare-root')).toMatchObject({ - installSource: 'someone/direct-root:bare-root', + installSource: 'https://github.com/someone/direct-root:bare-root', pluginSourceKind: 'extension-root', }); expect(discovered.find((p) => p.name === '2048-game')).toMatchObject({ @@ -530,7 +530,7 @@ describe('discoverPlugins', () => { convertedDir = converted.extensionDir; expect(plugin).toMatchObject({ - installSource: 'someone/qoder-plugin:qoder-alias', + installSource: 'https://github.com/someone/qoder-plugin:qoder-alias', pluginSourceKind: 'extension-root', }); expect(converted.originSource).toBe('Qoder'); @@ -580,8 +580,7 @@ describe('discoverPlugins', () => { it('rejects local-path sources from a remote (http) marketplace', async () => { // A hostile remote marketplace must not be able to point the installer at a - // local filesystem path — via either the bare-string source or the - // structured { source: 'url' } form. Both fall back to the plugin name. + // local filesystem path through any supported source shape. vi.mocked(loadMarketplaceConfigFromSource).mockResolvedValue( config('Remote', [ { name: 'abs', version: '1.0.0', source: '/etc/passwd' }, @@ -597,6 +596,21 @@ describe('discoverPlugins', () => { version: '1.0.0', source: { source: 'url', url: '../escape' }, }, + { + name: 'githubabs', + version: '1.0.0', + source: { source: 'github', repo: '/home/user/secrets' }, + }, + { + name: 'githubrel', + version: '1.0.0', + source: { source: 'github', repo: 'foo/bar/baz' }, + }, + { + name: 'githubcwd', + version: '1.0.0', + source: { source: 'github', repo: 'foo/bar' }, + }, { name: 'urlok', version: '1.0.0', @@ -619,6 +633,11 @@ describe('discoverPlugins', () => { // { source: 'url' } local paths are rejected too (previously bypassed). expect(src('urlabs')).toBe(''); expect(src('urlrel')).toBe(''); + expect(src('githubabs')).toBe(''); + expect(src('githubrel')).toBe(''); + // owner/repo is normalized before it reaches parseInstallSource, whose + // filesystem-first resolution would otherwise select ./foo/bar. + expect(src('githubcwd')).toBe('https://github.com/foo/bar:githubcwd'); // A genuine remote URL is preserved. expect(src('urlok')).toBe('https://example.com/p.tgz'); }); diff --git a/packages/core/src/extension/sourceRegistry.ts b/packages/core/src/extension/sourceRegistry.ts index b0b8dec908a..19a2db4eea2 100644 --- a/packages/core/src/extension/sourceRegistry.ts +++ b/packages/core/src/extension/sourceRegistry.ts @@ -158,7 +158,34 @@ function isGitHubHost(url: string): boolean { } function isOwnerRepoShorthand(source: string): boolean { - return /^[a-zA-Z0-9_.-]+\/[a-zA-Z0-9_.-]+$/.test(source); + const match = source.match(/^([a-zA-Z0-9_.-]+)\/([a-zA-Z0-9_.-]+)$/); + return Boolean( + match && + match[1] !== '.' && + match[1] !== '..' && + match[2] !== '.' && + match[2] !== '..', + ); +} + +/** + * Makes a plugin source from a remote marketplace unambiguously remote. + * + * `parseInstallSource` deliberately checks the filesystem before interpreting + * `owner/repo` shorthand. Leaving that shorthand untouched here would let a + * remote marketplace select a same-named directory below the process cwd. + */ +function normalizeRemotePluginSource(source: string): string | undefined { + const trimmed = source.trim(); + const { repo, pluginName } = parseSourceAndPluginName(trimmed); + if (parseExtensionSourceType(repo) === 'local') { + return undefined; + } + + const normalizedRepo = isOwnerRepoShorthand(repo) + ? `https://github.com/${repo}` + : repo; + return pluginName ? `${normalizedRepo}:${pluginName}` : normalizedRepo; } function classifyRemotePluginSource( @@ -212,38 +239,38 @@ function resolveInstallSource( } const src = plugin.source; if (typeof src === 'string') { - // A remote marketplace must not be able to point the installer at an - // arbitrary local filesystem path (e.g. "/opt/secret" or "../../etc"). - if (path.isAbsolute(src) || src.startsWith('.') || src.startsWith('~')) { + const normalizedSource = normalizeRemotePluginSource(src); + if (!normalizedSource) { debugLogger.warn( `Ignoring local path source "${src}" from remote marketplace "${marketplace.source}".`, ); return { installSource: '', pluginSourceKind: 'extension-root' }; } - const isDirectUrl = /^https?:\/\//i.test(src); + const isDirectUrl = /^https?:\/\//i.test(src.trim()); return classifyRemotePluginSource( - src, + normalizedSource, isDirectUrl ? undefined : plugin.name, ); } if (src && src.source === 'github' && typeof src.repo === 'string') { - return classifyRemotePluginSource(src.repo, plugin.name); + const normalizedSource = normalizeRemotePluginSource(src.repo); + if (!normalizedSource) { + debugLogger.warn( + `Ignoring local path source "${src.repo}" from remote marketplace "${marketplace.source}".`, + ); + return { installSource: '', pluginSourceKind: 'extension-root' }; + } + return classifyRemotePluginSource(normalizedSource, plugin.name); } if (src && src.source === 'url' && typeof src.url === 'string') { - // Same local-path guard as the string-source branch above: a remote - // marketplace must not be able to redirect the installer at a local - // filesystem path via the structured `{ source: 'url' }` form either. - if ( - path.isAbsolute(src.url) || - src.url.startsWith('.') || - src.url.startsWith('~') - ) { + const normalizedSource = normalizeRemotePluginSource(src.url); + if (!normalizedSource) { debugLogger.warn( `Ignoring local path source "${src.url}" from remote marketplace "${marketplace.source}".`, ); return { installSource: '', pluginSourceKind: 'extension-root' }; } - return classifyRemotePluginSource(src.url); + return classifyRemotePluginSource(normalizedSource); } // A direct JSON document has no cloneable root for a missing/relative source. // Keep the entry visible for provenance, but mark it non-installable instead From 39faca609840483fe46c38a0e11d42c378de903e Mon Sep 17 00:00:00 2001 From: destire-mio <248462155+destire-mio@users.noreply.github.com> Date: Sat, 22 Aug 2026 01:56:44 +0800 Subject: [PATCH 19/19] fix(extensions): parse numeric HTTPS plugin aliases --- .../core/src/extension/marketplace.test.ts | 27 ++++++++++++++++- packages/core/src/extension/marketplace.ts | 15 ++++++---- .../core/src/extension/sourceRegistry.test.ts | 30 +++++++++++++++++++ 3 files changed, 65 insertions(+), 7 deletions(-) diff --git a/packages/core/src/extension/marketplace.test.ts b/packages/core/src/extension/marketplace.test.ts index b1507ca4627..dd37b37ad0f 100644 --- a/packages/core/src/extension/marketplace.test.ts +++ b/packages/core/src/extension/marketplace.test.ts @@ -171,12 +171,37 @@ describe('parseInstallSource', () => { it('should not treat a trailing HTTPS port as a plugin name', async () => { vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); - const result = await parseInstallSource('https://example.com:8080'); + const result = await parseInstallSource('https://example.com:8080', { + pluginSourceKind: 'marketplace-entry', + }); expect(result.source).toBe('https://example.com:8080'); expect(result.pluginName).toBeUndefined(); }); + it('preserves a non-numeric HTTPS alias without a path', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + + const result = await parseInstallSource( + 'https://example.com:root-plugin', + ); + + expect(result.source).toBe('https://example.com'); + expect(result.pluginName).toBe('root-plugin'); + }); + + it('parses an all-digit HTTPS marketplace selector after the URL path', async () => { + vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); + + const result = await parseInstallSource( + 'https://github.com/owner/repo:2048', + ); + + expect(result.source).toBe('https://github.com/owner/repo'); + expect(result.pluginName).toBe('2048'); + expect(result.pluginSourceKind).toBe('marketplace-entry'); + }); + it('should parse an uppercase HTTPS URL scheme as a git source', async () => { // Mock stat to fail (not a local path) vi.mocked(fs.stat).mockRejectedValueOnce(new Error('ENOENT')); diff --git a/packages/core/src/extension/marketplace.ts b/packages/core/src/extension/marketplace.ts index 71610061267..e77136a84ac 100644 --- a/packages/core/src/extension/marketplace.ts +++ b/packages/core/src/extension/marketplace.ts @@ -63,15 +63,18 @@ export function parseSourceAndPluginName(source: string): { if (lastColonIndex !== -1) { // Check if what follows the colon looks like a pluginName (not a port number or path) const potentialPluginName = afterScheme.substring(lastColonIndex + 1); - // Plugin names may begin with digits (for example, "2048-game"). Only - // a fully numeric suffix is a port rather than an appended alias. + const separatorIndex = scheme.length + lastColonIndex; + const firstPathSlashIndex = source.indexOf('/', scheme.length); + const isHttpPort = + (scheme === 'http://' || scheme === 'https://') && + /^\d+$/.test(potentialPluginName) && + (firstPathSlashIndex === -1 || separatorIndex < firstPathSlashIndex); + // HTTP(S) ports appear in the authority, before the first path slash. + // A suffix after the path is an appended plugin name, even if numeric. if ( potentialPluginName && !potentialPluginName.includes('/') && - !( - (scheme === 'http://' || scheme === 'https://') && - /^\d+$/.test(potentialPluginName) - ) + !isHttpPort ) { repoEndIndex = scheme.length + lastColonIndex; hasPluginName = true; diff --git a/packages/core/src/extension/sourceRegistry.test.ts b/packages/core/src/extension/sourceRegistry.test.ts index 2e97bb5984e..c4c46adcbda 100644 --- a/packages/core/src/extension/sourceRegistry.test.ts +++ b/packages/core/src/extension/sourceRegistry.test.ts @@ -356,6 +356,16 @@ describe('discoverPlugins', () => { version: '1.0.0', source: 'git@github.com:someone/numeric-root.git', }, + { + name: '4096', + version: '1.0.0', + source: { source: 'github', repo: 'someone/numeric-https-root' }, + }, + { + name: 'embedded-numeric', + version: '1.0.0', + source: 'someone/repo:2048', + }, ]), ); @@ -455,6 +465,26 @@ describe('discoverPlugins', () => { repo: 'git@github.com:someone/numeric-root.git', pluginName: '2048', }); + const numericHttpsRoot = discovered.find((p) => p.name === '4096')!; + expect(numericHttpsRoot).toMatchObject({ + installSource: 'https://github.com/someone/numeric-https-root:4096', + pluginSourceKind: 'extension-root', + }); + expect(parseSourceAndPluginName(numericHttpsRoot.installSource)).toEqual({ + repo: 'https://github.com/someone/numeric-https-root', + pluginName: '4096', + }); + const embeddedNumeric = discovered.find( + (p) => p.name === 'embedded-numeric', + )!; + expect(embeddedNumeric).toMatchObject({ + installSource: 'https://github.com/someone/repo:2048', + pluginSourceKind: 'marketplace-entry', + }); + expect(parseSourceAndPluginName(embeddedNumeric.installSource)).toEqual({ + repo: 'https://github.com/someone/repo', + pluginName: '2048', + }); }); it.each([