diff --git a/packages/coding-agent/docs/packages.md b/packages/coding-agent/docs/packages.md index 6aa1d8afc..f2d1bc148 100644 --- a/packages/coding-agent/docs/packages.md +++ b/packages/coding-agent/docs/packages.md @@ -75,6 +75,8 @@ Use an explicit manifest when resources live elsewhere or need filtering: Paths are relative to the package root. Arrays accept glob patterns and exclusions. List dot-prefixed or symlinked resource roots directly when traversal through a glob would not discover them. +When an extension path resolves to a directory, Pi uses that directory's own `pi.extensions` manifest first. Otherwise it loads the directory's `package.json` `main`, then `index.ts` or `index.js`, before scanning direct extension files. A `.` entry does not re-read its containing manifest. + The `pi-package` keyword makes an npm package eligible for discovery in the [Pi package gallery](https://pi.dev/packages). Optional `pi.image` and `pi.video` fields add gallery previews. ## Show a changelog diff --git a/packages/coding-agent/src/core/extensions/loader.ts b/packages/coding-agent/src/core/extensions/loader.ts index 6c7cc6f99..23c8c9821 100644 --- a/packages/coding-agent/src/core/extensions/loader.ts +++ b/packages/coding-agent/src/core/extensions/loader.ts @@ -17,11 +17,10 @@ import { createEventBus, type EventBus } from "../event-bus.ts"; import type { ExecOptions } from "../exec.ts"; import { execCommand } from "../exec.ts"; import { type McpServerConfig, McpServerRegistry, validateMcpServerConfig } from "../mcp-servers.ts"; -import { readPiManifest } from "../pi-manifest.ts"; import { createSyntheticSourceInfo, getSyntheticPathSource, isSyntheticPath } from "../source-info.ts"; import { time } from "../timings.ts"; import type { ModelRouteRequest, VirtualModelDefinition } from "../virtual-models.ts"; -import { resolveExtensionManifestEntries } from "./manifest.ts"; +import { discoverExtensionDirectoryContents, discoverExtensionsInDir } from "./manifest.ts"; import type { EntryRenderer, Extension, @@ -746,84 +745,6 @@ export async function loadExtensionsCached( return loadExtensionsInternal(paths, cwd, eventBus, runtime, true); } -function isExtensionFile(name: string): boolean { - return name.endsWith(".ts") || name.endsWith(".js"); -} - -/** - * Resolve extension entry points from a directory. - * - * Checks for: - * 1. package.json with "pi.extensions" field -> returns declared paths - * 2. index.ts or index.js -> returns the index file - * - * Returns resolved paths or null if no entry points found. - */ -function resolveExtensionEntries(dir: string): string[] | null { - // Check for package.json with "pi" field first - const packageJsonPath = path.join(dir, "package.json"); - if (fs.existsSync(packageJsonPath)) { - const manifest = readPiManifest(packageJsonPath); - if (manifest?.extensions !== undefined) return resolveExtensionManifestEntries(dir, manifest.extensions); - } - - // Check for index.ts or index.js - const indexTs = path.join(dir, "index.ts"); - const indexJs = path.join(dir, "index.js"); - if (fs.existsSync(indexTs)) { - return [indexTs]; - } - if (fs.existsSync(indexJs)) { - return [indexJs]; - } - - return null; -} - -/** - * Discover extensions in a directory. - * - * Discovery rules: - * 1. Direct files: `extensions/*.ts` or `*.js` → load - * 2. Subdirectory with index: `extensions/* /index.ts` or `index.js` → load - * 3. Subdirectory with package.json: `extensions/* /package.json` with "pi" field → load what it declares - * - * No recursion beyond one level. Complex packages must use package.json manifest. - */ -function discoverExtensionsInDir(dir: string): string[] { - if (!fs.existsSync(dir)) { - return []; - } - - const discovered: string[] = []; - - try { - const entries = fs.readdirSync(dir, { withFileTypes: true }); - - for (const entry of entries) { - const entryPath = path.join(dir, entry.name); - - // 1. Direct files: *.ts or *.js - if ((entry.isFile() || entry.isSymbolicLink()) && isExtensionFile(entry.name)) { - discovered.push(entryPath); - continue; - } - - // 2 & 3. Subdirectories - if (entry.isDirectory() || entry.isSymbolicLink()) { - const entries = resolveExtensionEntries(entryPath); - if (entries) { - discovered.push(...entries); - } - } - } - } catch { - return []; - } - - return discovered; -} - /** * Discover and load extensions from standard locations. */ @@ -850,23 +771,16 @@ export async function discoverAndLoadExtensions( // 1. Project-local extensions: cwd/${CONFIG_DIR_NAME}/extensions/ const localExtDir = path.join(resolvedCwd, CONFIG_DIR_NAME, "extensions"); - addPaths(discoverExtensionsInDir(localExtDir)); + addPaths(discoverExtensionDirectoryContents(localExtDir)); // 2. Global extensions: agentDir/extensions/ const globalExtDir = path.join(resolvedAgentDir, "extensions"); - addPaths(discoverExtensionsInDir(globalExtDir)); + addPaths(discoverExtensionDirectoryContents(globalExtDir)); // 3. Explicitly configured paths for (const p of configuredPaths) { const resolved = resolvePath(p, resolvedCwd, { normalizeUnicodeSpaces: true }); if (fs.existsSync(resolved) && fs.statSync(resolved).isDirectory()) { - // Check for package.json with pi manifest or index.ts - const entries = resolveExtensionEntries(resolved); - if (entries) { - addPaths(entries); - continue; - } - // No explicit entries - discover individual files in directory addPaths(discoverExtensionsInDir(resolved)); continue; } diff --git a/packages/coding-agent/src/core/extensions/manifest.ts b/packages/coding-agent/src/core/extensions/manifest.ts index 5013df48b..4fb2d02ff 100644 --- a/packages/coding-agent/src/core/extensions/manifest.ts +++ b/packages/coding-agent/src/core/extensions/manifest.ts @@ -1,58 +1,189 @@ -import { existsSync, globSync } from "node:fs"; -import { basename, relative, resolve, sep } from "node:path"; +import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from "node:fs"; +import { basename, join, relative, resolve, sep } from "node:path"; +import ignore from "ignore"; import { minimatch } from "minimatch"; +import { + expandManifestGlob, + getPiManifest, + hasManifestGlobMagic, + type ManifestPattern, + normalizeManifestPattern, + type PackageJson, + readPackageJson, + resolveManifestPath, +} from "../pi-manifest.ts"; -function normalizePattern(pattern: string): string { - const normalized = pattern.startsWith("./") || pattern.startsWith(".\\") ? pattern.slice(2) : pattern; - return normalized.split(sep).join("/"); +const IGNORE_FILE_NAMES = [".gitignore", ".ignore", ".fdignore"]; + +export class ExtensionManifestResolutionError extends Error { + readonly code = "EXTENSION_MANIFEST_CYCLE"; + readonly manifestRoot: string; + + constructor(manifestRoot: string) { + super(`Cyclic extension manifest reference: ${manifestRoot}`); + this.name = "ExtensionManifestResolutionError"; + this.manifestRoot = manifestRoot; + } } -function isOverride(entry: string): boolean { - return entry.startsWith("!") || entry.startsWith("+") || entry.startsWith("-"); -} - -function getMatchCandidates(entryPath: string, root: string): { glob: string[]; exact: string[] } { - const relativePath = relative(root, entryPath).split(sep).join("/"); - const absolutePath = entryPath.split(sep).join("/"); +function getMatchCandidates(entryPath: string, root: string) { return { - glob: [relativePath, basename(entryPath), absolutePath], - exact: [relativePath, absolutePath], + relative: relative(root, entryPath).split(sep).join("/"), + name: basename(entryPath), + absolute: entryPath.split(sep).join("/"), }; } -/** Resolve an extension manifest declaration to its selected existing paths. */ -export function resolveExtensionManifestEntries(root: string, entries: string[]): string[] { - const exclusions = entries.filter((entry) => entry.startsWith("!")).map((entry) => normalizePattern(entry.slice(1))); +function matchesPattern(entryPath: string, pattern: ManifestPattern, root: string): boolean { + const candidates = getMatchCandidates(entryPath, root); + return ( + minimatch(candidates.relative, pattern.value) || + (!pattern.rootAnchored && minimatch(candidates.name, pattern.value)) || + minimatch(candidates.absolute, pattern.value) + ); +} + +function matchesExactPath(entryPath: string, pattern: ManifestPattern, root: string): boolean { + const candidates = getMatchCandidates(entryPath, root); + return candidates.relative === pattern.value || candidates.absolute === pattern.value; +} + +function createIgnoreMatcher(dir: string): ReturnType { + const matcher = ignore(); + for (const filename of IGNORE_FILE_NAMES) { + const ignorePath = join(dir, filename); + if (existsSync(ignorePath)) matcher.add(readFileSync(ignorePath, "utf-8")); + } + return matcher; +} + +function resolveModulePath(entryPath: string): string | null { + for (const candidate of [ + entryPath, + `${entryPath}.ts`, + `${entryPath}.js`, + join(entryPath, "index.ts"), + join(entryPath, "index.js"), + ]) { + if (statSync(candidate, { throwIfNoEntry: false })?.isFile()) return candidate; + } + return null; +} + +function resolveDirectoryModuleEntry(dir: string, packageJson: PackageJson | null): string | null { + const main = packageJson?.main; + if (typeof main === "string" && main.length > 0) { + const mainEntry = resolveModulePath(resolve(dir, main)); + if (mainEntry) return mainEntry; + } + + return resolveModulePath(join(dir, "index")); +} + +function resolveExtensionPackageInternal(dir: string, manifestRoots: Set): string[] | null { + const packageJson = readPackageJson(join(dir, "package.json")); + const extensions = packageJson ? getPiManifest(packageJson)?.extensions : undefined; + if (packageJson && extensions !== undefined) { + const manifestRoot = realpathSync(dir); + if (manifestRoots.has(manifestRoot)) throw new ExtensionManifestResolutionError(manifestRoot); + manifestRoots.add(manifestRoot); + try { + return resolveExtensionManifestEntriesInternal(dir, extensions, manifestRoots, packageJson); + } finally { + manifestRoots.delete(manifestRoot); + } + } + + const moduleEntry = resolveDirectoryModuleEntry(dir, packageJson); + return moduleEntry ? [moduleEntry] : null; +} + +function discoverExtensionsInDirInternal(dir: string, manifestRoots: Set): string[] { + if (!existsSync(dir)) return []; + return ( + resolveExtensionPackageInternal(dir, manifestRoots) ?? + discoverExtensionDirectoryContentsInternal(dir, manifestRoots) + ); +} + +function discoverExtensionDirectoryContentsInternal(dir: string, manifestRoots: Set): string[] { + if (!existsSync(dir)) return []; + const matcher = createIgnoreMatcher(dir); + const extensions: string[] = []; + + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (entry.name.startsWith(".") || entry.name === "node_modules") continue; + const entryPath = join(dir, entry.name); + let isDirectory = entry.isDirectory(); + let isFile = entry.isFile(); + if (entry.isSymbolicLink()) { + try { + const stats = statSync(entryPath); + isDirectory = stats.isDirectory(); + isFile = stats.isFile(); + } catch (error) { + if (error instanceof Error && "code" in error && error.code === "ENOENT") continue; + throw error; + } + } + if (matcher.ignores(isDirectory ? `${entry.name}/` : entry.name)) continue; + + if (isFile && (entry.name.endsWith(".ts") || entry.name.endsWith(".js"))) { + extensions.push(entryPath); + } else if (isDirectory) { + const packageEntries = resolveExtensionPackageInternal(entryPath, manifestRoots); + if (packageEntries) extensions.push(...packageEntries); + } + } + return extensions; +} + +function resolveExtensionManifestEntriesInternal( + root: string, + entries: string[], + manifestRoots: Set, + packageJson: PackageJson, +): string[] { + const exclusions = entries + .filter((entry) => entry.startsWith("!")) + .map((entry) => normalizeManifestPattern(entry.slice(1))); const forceIncludes = entries .filter((entry) => entry.startsWith("+")) - .map((entry) => normalizePattern(entry.slice(1))); + .map((entry) => normalizeManifestPattern(entry.slice(1))); const forceExcludes = entries .filter((entry) => entry.startsWith("-")) - .map((entry) => normalizePattern(entry.slice(1))); - const discovered = entries - .filter((entry) => !isOverride(entry)) - .flatMap((entry) => { - if (!entry.includes("*") && !entry.includes("?")) return [resolve(root, entry)]; - return globSync(entry, { cwd: root }) - .map((match) => resolve(root, match)) - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); - }) - .filter(existsSync); - const selected = new Set( - discovered.filter((entry) => { - const candidates = getMatchCandidates(entry, root).glob; - return !exclusions.some((pattern) => candidates.some((candidate) => minimatch(candidate, pattern))); - }), + .map((entry) => normalizeManifestPattern(entry.slice(1))); + const sourcePaths = entries + .filter((entry) => !entry.startsWith("!") && !entry.startsWith("+") && !entry.startsWith("-")) + .flatMap((entry) => + hasManifestGlobMagic(entry) ? expandManifestGlob(entry, root) : [resolveManifestPath(entry, root)], + ); + const resolvedRoot = resolve(root); + const discovered = Array.from( + new Set( + sourcePaths.flatMap((entryPath) => { + if (!existsSync(entryPath)) return []; + if (!statSync(entryPath).isDirectory()) return [entryPath]; + if (entryPath !== resolvedRoot) return discoverExtensionsInDirInternal(entryPath, manifestRoots); + const moduleEntry = resolveDirectoryModuleEntry(entryPath, packageJson); + return moduleEntry ? [moduleEntry] : discoverExtensionDirectoryContentsInternal(entryPath, manifestRoots); + }), + ), + ); + return discovered.filter( + (entry) => + !forceExcludes.some((pattern) => matchesExactPath(entry, pattern, root)) && + (forceIncludes.some((pattern) => matchesExactPath(entry, pattern, root)) || + !exclusions.some((pattern) => matchesPattern(entry, pattern, root))), ); - - for (const entry of discovered) { - const candidates = getMatchCandidates(entry, root).exact; - if (forceIncludes.some((pattern) => candidates.includes(pattern))) selected.add(entry); - } - for (const entry of selected) { - const candidates = getMatchCandidates(entry, root).exact; - if (forceExcludes.some((pattern) => candidates.includes(pattern))) selected.delete(entry); - } - - return discovered.filter((entry) => selected.has(entry)); +} + +/** Discover a directory as one extension package, falling back to its extension contents. */ +export function discoverExtensionsInDir(dir: string): string[] { + return discoverExtensionsInDirInternal(dir, new Set()); +} + +/** Discover direct extension files and immediate extension package directories. */ +export function discoverExtensionDirectoryContents(dir: string): string[] { + return discoverExtensionDirectoryContentsInternal(dir, new Set()); } diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 30a5b0eb8..e00323f80 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -3,7 +3,6 @@ import { createHash } from "node:crypto"; import { chmodSync, existsSync, - globSync, mkdirSync, readdirSync, readFileSync, @@ -51,9 +50,16 @@ import { resolvePath, } from "../utils/paths.ts"; import { stripBom } from "../utils/text.ts"; -import { resolveExtensionManifestEntries } from "./extensions/manifest.ts"; +import { discoverExtensionsInDir, ExtensionManifestResolutionError } from "./extensions/manifest.ts"; import { isStdoutTakenOver } from "./output-guard.ts"; -import { type PiManifest, readPiManifest } from "./pi-manifest.ts"; +import { + expandManifestGlob, + hasManifestGlobMagic, + normalizeManifestPattern, + type PiManifest, + readPiManifest, + resolveManifestPath, +} from "./pi-manifest.ts"; import type { PackageSource, SettingsManager } from "./settings-manager.ts"; import { BUILTIN_PATH_PREFIX, getResourcePathKey } from "./source-info.ts"; @@ -326,22 +332,7 @@ function isOverridePattern(s: string): boolean { return s.startsWith("!") || s.startsWith("+") || s.startsWith("-"); } -function hasGlobPattern(s: string): boolean { - return s.includes("*") || s.includes("?"); -} - /** Glob entries discover visible paths; exact entries can target dot paths or symlinked trees. */ -function expandPackageGlob(pattern: string, root: string): string[] { - return globSync(pattern, { cwd: root }) - .map((match) => resolve(root, match)) - .filter((path) => - relative(root, path) - .split(sep) - .every((segment) => segment === ".." || !segment.startsWith(".")), - ) - .sort((a, b) => (a < b ? -1 : a > b ? 1 : 0)); -} - function splitPatterns(entries: string[]): { plain: string[]; patterns: string[] } { const plain: string[] = []; const patterns: string[] = []; @@ -600,79 +591,6 @@ function collectAutoThemeEntries(dir: string): string[] { return entries; } -function resolveExtensionEntries(dir: string): string[] | null { - const packageJsonPath = join(dir, "package.json"); - if (existsSync(packageJsonPath)) { - const manifest = readPiManifest(packageJsonPath); - if (manifest?.extensions !== undefined) return resolveExtensionManifestEntries(dir, manifest.extensions); - } - - const indexTs = join(dir, "index.ts"); - const indexJs = join(dir, "index.js"); - if (existsSync(indexTs)) { - return [indexTs]; - } - if (existsSync(indexJs)) { - return [indexJs]; - } - - return null; -} - -function collectAutoExtensionEntries(dir: string): string[] { - const entries: string[] = []; - if (!existsSync(dir)) return entries; - - // First check if this directory itself has explicit extension entries (package.json or index) - const rootEntries = resolveExtensionEntries(dir); - if (rootEntries) { - return rootEntries; - } - - // Otherwise, discover extensions from directory contents - const ig = ignore(); - addIgnoreRules(ig, dir, dir); - - try { - const dirEntries = readdirSync(dir, { withFileTypes: true }); - for (const entry of dirEntries) { - if (entry.name.startsWith(".")) continue; - if (entry.name === "node_modules") continue; - - const fullPath = join(dir, entry.name); - let isDir = entry.isDirectory(); - let isFile = entry.isFile(); - - if (entry.isSymbolicLink()) { - try { - const stats = statSync(fullPath); - isDir = stats.isDirectory(); - isFile = stats.isFile(); - } catch { - continue; - } - } - - const relPath = toPosixPath(relative(dir, fullPath)); - const ignorePath = isDir ? `${relPath}/` : relPath; - if (ig.ignores(ignorePath)) continue; - - if (isFile && (entry.name.endsWith(".ts") || entry.name.endsWith(".js"))) { - entries.push(fullPath); - } else if (isDir) { - const resolvedEntries = resolveExtensionEntries(fullPath); - if (resolvedEntries) { - entries.push(...resolvedEntries); - } - } - } - } catch { - // Ignore errors - } - - return entries; -} - /** * Collect resource files from a directory based on resource type. * Extensions use smart discovery (index.ts in subdirs), others use recursive collection. @@ -682,7 +600,7 @@ function collectResourceFiles(dir: string, resourceType: ResourceType): string[] return collectSkillEntries(dir, "pi"); } if (resourceType === "extensions") { - return collectAutoExtensionEntries(dir); + return discoverExtensionsInDir(dir); } return collectFiles(dir, FILE_PATTERNS[resourceType]); } @@ -718,12 +636,20 @@ function resolvePackageChangelogPath(packageRoot: string, manifest: PiManifest | } } -function normalizePattern(pattern: string): string { - const normalized = pattern.startsWith("./") || pattern.startsWith(".\\") ? pattern.slice(2) : pattern; - return toPosixPath(normalizePath(normalized)); +function normalizePattern(pattern: string): { value: string; rootAnchored: boolean } { + const rootAnchored = pattern.startsWith("./") || pattern.startsWith(".\\"); + const normalized = rootAnchored ? pattern.slice(2) : pattern; + return { value: toPosixPath(normalizePath(normalized)), rootAnchored }; } -function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string): boolean { +type PatternNormalizer = (pattern: string) => { value: string; rootAnchored: boolean }; + +function matchesAnyPattern( + filePath: string, + patterns: string[], + baseDir: string, + normalize: PatternNormalizer = normalizePattern, +): boolean { const rel = toPosixPath(relative(baseDir, filePath)); const name = basename(filePath); const filePathPosix = toPosixPath(filePath); @@ -734,10 +660,10 @@ function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string const parentDirPosix = isSkillFile ? toPosixPath(parentDir!) : undefined; return patterns.some((pattern) => { - const normalizedPattern = normalizePattern(pattern); + const { value: normalizedPattern, rootAnchored } = normalize(pattern); if ( minimatch(rel, normalizedPattern) || - minimatch(name, normalizedPattern) || + (!rootAnchored && minimatch(name, normalizedPattern)) || minimatch(filePathPosix, normalizedPattern) ) { return true; @@ -745,13 +671,18 @@ function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string if (!isSkillFile) return false; return ( minimatch(parentRel!, normalizedPattern) || - minimatch(parentName!, normalizedPattern) || + (!rootAnchored && minimatch(parentName!, normalizedPattern)) || minimatch(parentDirPosix!, normalizedPattern) ); }); } -function matchesAnyExactPattern(filePath: string, patterns: string[], baseDir: string): boolean { +function matchesAnyExactPattern( + filePath: string, + patterns: string[], + baseDir: string, + normalize: PatternNormalizer = normalizePattern, +): boolean { if (patterns.length === 0) return false; const rel = toPosixPath(relative(baseDir, filePath)); const name = basename(filePath); @@ -762,7 +693,7 @@ function matchesAnyExactPattern(filePath: string, patterns: string[], baseDir: s const parentDirPosix = isSkillFile ? toPosixPath(parentDir!) : undefined; return patterns.some((pattern) => { - const normalized = normalizePattern(pattern); + const { value: normalized } = normalize(pattern); if (normalized === rel || normalized === filePathPosix) { return true; } @@ -802,7 +733,12 @@ function isEnabledByOverrides(filePath: string, patterns: string[], baseDir: str * - `+path`: force-include exact path (overrides exclusions) * - `-path`: force-exclude exact path (overrides force-includes) */ -function applyPatterns(allPaths: string[], patterns: string[], baseDir: string): Set { +function applyPatterns( + allPaths: string[], + patterns: string[], + baseDir: string, + normalize: PatternNormalizer = normalizePattern, +): Set { const includes: string[] = []; const excludes: string[] = []; const forceIncludes: string[] = []; @@ -825,18 +761,18 @@ function applyPatterns(allPaths: string[], patterns: string[], baseDir: string): if (includes.length === 0) { result = [...allPaths]; } else { - result = allPaths.filter((filePath) => matchesAnyPattern(filePath, includes, baseDir)); + result = allPaths.filter((filePath) => matchesAnyPattern(filePath, includes, baseDir, normalize)); } // Step 2: Apply excludes if (excludes.length > 0) { - result = result.filter((filePath) => !matchesAnyPattern(filePath, excludes, baseDir)); + result = result.filter((filePath) => !matchesAnyPattern(filePath, excludes, baseDir, normalize)); } // Step 3: Force-include (add back from allPaths, overriding exclusions) if (forceIncludes.length > 0) { for (const filePath of allPaths) { - if (!result.includes(filePath) && matchesAnyExactPattern(filePath, forceIncludes, baseDir)) { + if (!result.includes(filePath) && matchesAnyExactPattern(filePath, forceIncludes, baseDir, normalize)) { result.push(filePath); } } @@ -844,7 +780,7 @@ function applyPatterns(allPaths: string[], patterns: string[], baseDir: string): // Step 4: Force-exclude (remove even if included or force-included) if (forceExcludes.length > 0) { - result = result.filter((filePath) => !matchesAnyExactPattern(filePath, forceExcludes, baseDir)); + result = result.filter((filePath) => !matchesAnyExactPattern(filePath, forceExcludes, baseDir, normalize)); } return new Set(result); @@ -1547,8 +1483,12 @@ export class DefaultPackageManager implements PackageManager { this.addResource(accumulator.extensions, resolved, metadata, true); } } - } catch { - return; + } catch (error) { + if (error instanceof ExtensionManifestResolutionError && error.code === "EXTENSION_MANIFEST_CYCLE") { + throw error; + } + if (error instanceof Error && "code" in error && error.code === "ENOENT") return; + throw error; } } @@ -2540,7 +2480,9 @@ export class DefaultPackageManager implements PackageManager { const allFiles = this.collectFilesFromManifestEntries(entries, packageRoot, resourceType); const manifestPatterns = entries.filter(isOverridePattern); const enabledByManifest = - manifestPatterns.length > 0 ? applyPatterns(allFiles, manifestPatterns, packageRoot) : new Set(allFiles); + manifestPatterns.length > 0 + ? applyPatterns(allFiles, manifestPatterns, packageRoot, normalizeManifestPattern) + : new Set(allFiles); return { allFiles: Array.from(enabledByManifest), enabledByManifest }; } @@ -2563,7 +2505,7 @@ export class DefaultPackageManager implements PackageManager { const allFiles = this.collectFilesFromManifestEntries(entries, root, resourceType); const patterns = entries.filter(isOverridePattern); - const enabledPaths = applyPatterns(allFiles, patterns, root); + const enabledPaths = applyPatterns(allFiles, patterns, root, normalizeManifestPattern); for (const f of allFiles) { if (enabledPaths.has(f)) { @@ -2575,11 +2517,11 @@ export class DefaultPackageManager implements PackageManager { private collectFilesFromManifestEntries(entries: string[], root: string, resourceType: ResourceType): string[] { const sourceEntries = entries.filter((entry) => !isOverridePattern(entry)); const resolved = sourceEntries.flatMap((entry) => { - if (!hasGlobPattern(entry)) { - return [resolve(root, entry)]; + if (!hasManifestGlobMagic(entry)) { + return [resolveManifestPath(entry, root)]; } - return expandPackageGlob(entry, root); + return expandManifestGlob(entry, root); }); return this.collectFilesFromPaths(resolved, resourceType); } @@ -2676,7 +2618,7 @@ export class DefaultPackageManager implements PackageManager { // Project extensions from .pi/ addResources( "extensions", - collectAutoExtensionEntries(projectDirs.extensions), + discoverExtensionsInDir(projectDirs.extensions), projectMetadata, projectOverrides.extensions, projectBaseDir, @@ -2728,7 +2670,7 @@ export class DefaultPackageManager implements PackageManager { // User extensions from ~/.pi/agent/ addResources( "extensions", - collectAutoExtensionEntries(userDirs.extensions), + discoverExtensionsInDir(userDirs.extensions), userMetadata, userOverrides.extensions, globalBaseDir, @@ -2785,8 +2727,9 @@ export class DefaultPackageManager implements PackageManager { } else if (stats.isDirectory()) { files.push(...collectResourceFiles(p, resourceType)); } - } catch { - // Ignore errors + } catch (error) { + if (error instanceof Error && "code" in error && error.code === "ENOENT") continue; + throw error; } } return files; diff --git a/packages/coding-agent/src/core/pi-manifest.ts b/packages/coding-agent/src/core/pi-manifest.ts index 0dca39385..ebdee72d2 100644 --- a/packages/coding-agent/src/core/pi-manifest.ts +++ b/packages/coding-agent/src/core/pi-manifest.ts @@ -1,4 +1,6 @@ -import { readFileSync } from "node:fs"; +import { globSync, readFileSync } from "node:fs"; +import { relative, resolve, sep } from "node:path"; +import { Minimatch } from "minimatch"; import { stripBom } from "../utils/text.ts"; export interface PiManifest { @@ -9,34 +11,77 @@ export interface PiManifest { changelogPath?: string; } -const RESOURCE_FIELDS = ["extensions", "skills", "prompts", "themes"] as const; +export type PackageJson = Record; -function isObject(value: unknown): value is Record { +export interface ManifestPattern { + value: string; + rootAnchored: boolean; +} + +const RESOURCE_FIELDS = ["extensions", "skills", "prompts", "themes"] as const; +const MANIFEST_GLOB_OPTIONS = { magicalBraces: true, windowsPathsNoEscape: true }; + +export function normalizeManifestPattern(pattern: string): ManifestPattern { + const rootAnchored = pattern.startsWith("./") || pattern.startsWith(".\\"); + const value = rootAnchored ? pattern.slice(2) : pattern; + return { value: value.replaceAll("\\", "/"), rootAnchored }; +} + +export function hasManifestGlobMagic(pattern: string): boolean { + return new Minimatch(pattern, MANIFEST_GLOB_OPTIONS).hasMagic(); +} + +export function resolveManifestPath(pattern: string, root: string): string { + return resolve(root, pattern.replaceAll("\\", "/")); +} + +/** Glob entries discover visible paths; exact entries can target dot paths or symlinked trees. */ +export function expandManifestGlob(pattern: string, root: string): string[] { + return globSync(pattern.replaceAll("\\", "/"), { cwd: root }) + .map((match) => resolve(root, match)) + .filter((path) => + relative(root, path) + .split(sep) + .every((segment) => segment === ".." || !segment.startsWith(".")), + ) + .sort(); +} + +function isObject(value: unknown): value is PackageJson { return typeof value === "object" && value !== null && !Array.isArray(value); } -export function readPiManifest(packageJsonPath: string): PiManifest | null { +export function readPackageJson(packageJsonPath: string): PackageJson | null { + let contents: string; try { - const pkg: unknown = JSON.parse(stripBom(readFileSync(packageJsonPath, "utf-8"))); - if (!isObject(pkg) || !isObject(pkg.pi)) { - return null; - } - - const manifest: PiManifest = {}; - for (const field of RESOURCE_FIELDS) { - const entries = pkg.pi[field]; - if (Array.isArray(entries) && entries.every((entry) => typeof entry === "string")) { - manifest[field] = entries; - } - } - - const changelogPath = pkg.pi.changelogPath; - if (typeof changelogPath === "string") { - manifest.changelogPath = changelogPath; - } - - return manifest; - } catch { - return null; + contents = readFileSync(packageJsonPath, "utf-8"); + } catch (error) { + if (error instanceof Error && "code" in error && error.code === "ENOENT") return null; + throw error; } + + const packageJson: unknown = JSON.parse(stripBom(contents)); + return isObject(packageJson) ? packageJson : null; +} + +export function getPiManifest(packageJson: PackageJson): PiManifest | null { + if (!isObject(packageJson.pi)) return null; + + const manifest: PiManifest = {}; + for (const field of RESOURCE_FIELDS) { + const entries = packageJson.pi[field]; + if (Array.isArray(entries) && entries.every((entry) => typeof entry === "string")) { + manifest[field] = entries; + } + } + + const changelogPath = packageJson.pi.changelogPath; + if (typeof changelogPath === "string") manifest.changelogPath = changelogPath; + + return manifest; +} + +export function readPiManifest(packageJsonPath: string): PiManifest | null { + const packageJson = readPackageJson(packageJsonPath); + return packageJson ? getPiManifest(packageJson) : null; } diff --git a/packages/coding-agent/test/extensions-discovery.test.ts b/packages/coding-agent/test/extensions-discovery.test.ts index 392525222..8a146f23e 100644 --- a/packages/coding-agent/test/extensions-discovery.test.ts +++ b/packages/coding-agent/test/extensions-discovery.test.ts @@ -4,6 +4,7 @@ import * as path from "node:path"; import { fileURLToPath } from "node:url"; import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { discoverAndLoadExtensions } from "../src/core/extensions/loader.ts"; +import { discoverExtensionsInDir } from "../src/core/extensions/manifest.ts"; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -132,6 +133,17 @@ describe("extensions discovery", () => { expect(path.basename(result.extensions[0].path)).toBe("foo.js"); }); + it.skipIf(process.platform === "win32")("skips dangling extension symlinks", async () => { + const extensionPath = path.join(extensionsDir, "main.ts"); + fs.writeFileSync(extensionPath, extensionCode); + fs.symlinkSync(path.join(extensionsDir, "missing.ts"), path.join(extensionsDir, "dangling.ts")); + + const result = await discoverAndLoadExtensions([], tempDir, tempDir); + + expect(result.errors).toEqual([]); + expect(result.extensions.map((extension) => extension.path)).toEqual([extensionPath]); + }); + it("discovers subdirectory with index.ts", async () => { const subdir = path.join(extensionsDir, "my-extension"); fs.mkdirSync(subdir); @@ -277,6 +289,103 @@ describe("extensions discovery", () => { expect(result.extensions).toEqual([]); }); + it("applies manifest exclusions to entry points resolved from directory globs", async () => { + const subdir = path.join(extensionsDir, "grouped-manifest-package"); + const activeIndex = path.join(subdir, "groups", "active", "index.ts"); + const legacyIndex = path.join(subdir, "groups", "legacy", "index.ts"); + fs.mkdirSync(path.dirname(activeIndex), { recursive: true }); + fs.mkdirSync(path.dirname(legacyIndex), { recursive: true }); + fs.writeFileSync(activeIndex, extensionCode); + fs.writeFileSync(legacyIndex, extensionCode); + fs.writeFileSync( + path.join(subdir, "package.json"), + JSON.stringify({ pi: { extensions: ["./groups/*/", "!./groups/legacy/index.ts"] } }), + ); + + const result = await discoverAndLoadExtensions([], tempDir, tempDir); + + expect(result.errors).toEqual([]); + expect(result.extensions.map((extension) => extension.path)).toEqual([activeIndex]); + }); + + it("loads direct extension files from manifest directories before applying exclusions", async () => { + const subdir = path.join(extensionsDir, "directory-manifest-package"); + const entriesDir = path.join(subdir, "entries"); + const extensionPath = path.join(entriesDir, "main.ts"); + fs.mkdirSync(entriesDir, { recursive: true }); + fs.writeFileSync(extensionPath, extensionCode); + fs.writeFileSync(path.join(entriesDir, "main.test.ts"), "throw new Error('test extension loaded');"); + fs.writeFileSync( + path.join(subdir, "package.json"), + JSON.stringify({ pi: { extensions: ["./entries", "!./entries/*.test.ts"] } }), + ); + + const result = await discoverAndLoadExtensions([], tempDir, tempDir); + + expect(result.errors).toEqual([]); + expect(result.extensions.map((extension) => extension.path)).toEqual([extensionPath]); + }); + + it("rejects mutually recursive extension manifests", async () => { + const firstDir = path.join(extensionsDir, "first-cycle-package"); + const secondDir = path.join(extensionsDir, "second-cycle-package"); + fs.mkdirSync(firstDir); + fs.mkdirSync(secondDir); + fs.writeFileSync( + path.join(firstDir, "package.json"), + JSON.stringify({ pi: { extensions: ["../second-cycle-package"] } }), + ); + fs.writeFileSync( + path.join(secondDir, "package.json"), + JSON.stringify({ pi: { extensions: ["../first-cycle-package"] } }), + ); + + await expect(discoverAndLoadExtensions([], tempDir, tempDir)).rejects.toMatchObject({ + code: "EXTENSION_MANIFEST_CYCLE", + }); + }); + + it("resolves backslash-separated manifest globs", () => { + // Regression test for #9788. + const subdir = path.join(extensionsDir, "backslash-glob-package"); + const mainPath = path.join(subdir, "main.ts"); + fs.mkdirSync(subdir); + fs.writeFileSync(mainPath, extensionCode); + fs.writeFileSync(path.join(subdir, "main.test.ts"), extensionCode); + fs.writeFileSync( + path.join(subdir, "package.json"), + JSON.stringify({ pi: { extensions: [".\\*.ts", "!.\\*.test.ts"] } }), + ); + + expect(discoverExtensionsInDir(subdir)).toEqual([mainPath]); + }); + + it("prefers index.ts when a manifest declares its own directory", () => { + // Regression test for #9788. + const subdir = path.join(extensionsDir, "self-directory-package"); + const indexPath = path.join(subdir, "index.ts"); + fs.mkdirSync(subdir); + fs.writeFileSync(indexPath, extensionCode); + fs.writeFileSync(path.join(subdir, "helper.ts"), extensionCode); + fs.writeFileSync(path.join(subdir, "package.json"), JSON.stringify({ pi: { extensions: ["."] } })); + + expect(discoverExtensionsInDir(subdir)).toEqual([indexPath]); + }); + + it("prefers a child package main entry over scanning its contents", () => { + // Regression test for #9788. + const subdir = path.join(extensionsDir, "child-main-package"); + const childDir = path.join(subdir, "child"); + const mainPath = path.join(childDir, "dist", "main.js"); + fs.mkdirSync(path.dirname(mainPath), { recursive: true }); + fs.writeFileSync(mainPath, extensionCode); + fs.writeFileSync(path.join(childDir, "helper.ts"), extensionCode); + fs.writeFileSync(path.join(childDir, "package.json"), JSON.stringify({ main: "./dist/main.js" })); + fs.writeFileSync(path.join(subdir, "package.json"), JSON.stringify({ pi: { extensions: ["./child"] } })); + + expect(discoverExtensionsInDir(subdir)).toEqual([mainPath]); + }); + it("package.json with pi field takes precedence over index.ts", async () => { const subdir = path.join(extensionsDir, "my-package"); fs.mkdirSync(subdir); diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index 803704964..3e3a8d4e1 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -1,7 +1,7 @@ import { EventEmitter } from "node:events"; import { existsSync, mkdirSync, realpathSync, rmSync, statSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; -import { basename, join, relative } from "node:path"; +import { basename, dirname, join, relative } from "node:path"; import { PassThrough } from "node:stream"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { DefaultPackageManager, type ProgressEvent, type ResolvedResource } from "../src/core/package-manager.ts"; @@ -188,6 +188,160 @@ describe("DefaultPackageManager", () => { expect(names(explicitlyResolved.extensions)).toEqual(expectedNames); }); + it("should keep dot-relative exclusions anchored to the package root", async () => { + const packageDir = join(tempDir, "root-anchored-manifest"); + const rootPrompt = join(packageDir, "foo.md"); + const nestedPrompt = join(packageDir, "nested", "foo.md"); + mkdirSync(dirname(nestedPrompt), { recursive: true }); + writeFileSync(rootPrompt, "Root prompt"); + writeFileSync(nestedPrompt, "Nested prompt"); + writeFileSync(join(packageDir, "package.json"), JSON.stringify({ pi: { prompts: [".", "!./foo.md"] } })); + + const result = await packageManager.resolveExtensionSources([packageDir]); + + expect(result.prompts.map((prompt) => prompt.path)).toEqual([nestedPrompt]); + }); + + it("should omit hidden glob matches consistently", async () => { + const extensionDir = join(agentDir, "extensions", "hidden-manifest-extension"); + mkdirSync(extensionDir, { recursive: true }); + writeFileSync(join(extensionDir, "main.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, ".hidden.ts"), "export default function() {}"); + writeFileSync( + join(extensionDir, "package.json"), + JSON.stringify({ pi: { extensions: ["./*.ts", "./.*.ts"] } }), + ); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const names = (resources: ResolvedResource[]) => resources.map((resource) => basename(resource.path)).sort(); + + expect(names(autoDiscovered.extensions)).toEqual(["main.ts"]); + expect(names(explicitlyResolved.extensions)).toEqual(["main.ts"]); + }); + + it("should resolve extension directories consistently", async () => { + const extensionDir = join(agentDir, "extensions", "directory-manifest-extension"); + const entriesDir = join(extensionDir, "entries"); + mkdirSync(entriesDir, { recursive: true }); + writeFileSync(join(entriesDir, "main.ts"), "export default function() {}"); + writeFileSync(join(entriesDir, "main.test.ts"), "export default function() {}"); + writeFileSync( + join(extensionDir, "package.json"), + JSON.stringify({ pi: { extensions: ["./entries", "!./entries/*.test.ts"] } }), + ); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const names = (resources: ResolvedResource[]) => resources.map((resource) => basename(resource.path)).sort(); + + expect(names(autoDiscovered.extensions)).toEqual(["main.ts"]); + expect(names(explicitlyResolved.extensions)).toEqual(["main.ts"]); + }); + + it.each([ + ["exact", ["~/main.ts", "!~/main.ts"], []], + ["glob", ["~/*.ts", "!~/*.test.ts"], ["main.ts"]], + ] as const)("should keep package-relative tilde %s patterns consistent", async (kind, entries, expectedNames) => { + // Regression test for #9788. + const extensionDir = join(agentDir, "extensions", `tilde-pattern-${kind}`); + mkdirSync(join(extensionDir, "~"), { recursive: true }); + writeFileSync(join(extensionDir, "~", "main.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, "~", "main.test.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, "package.json"), JSON.stringify({ pi: { extensions: entries } })); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const names = (resources: ResolvedResource[]) => resources.map((resource) => basename(resource.path)).sort(); + + expect(names(autoDiscovered.extensions)).toEqual(expectedNames); + expect(names(explicitlyResolved.extensions)).toEqual(expectedNames); + }); + + it.each([ + ["bracket", "./[ab].ts", ["a.ts", "b.ts"]], + ["brace", "./{foo,bar}.ts", ["bar.ts", "foo.ts"]], + ["extglob", "./@(foo|bar).ts", ["bar.ts", "foo.ts"]], + ] as const)("should resolve %s manifest glob syntax consistently", async (kind, pattern, expectedNames) => { + // Regression test for #9788. + const extensionDir = join(agentDir, "extensions", `glob-syntax-${kind}`); + mkdirSync(extensionDir, { recursive: true }); + for (const name of ["a.ts", "b.ts", "foo.ts", "bar.ts"]) { + writeFileSync(join(extensionDir, name), "export default function() {}"); + } + writeFileSync(join(extensionDir, "package.json"), JSON.stringify({ pi: { extensions: [pattern] } })); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const names = (resources: ResolvedResource[]) => resources.map((resource) => basename(resource.path)).sort(); + + expect(names(autoDiscovered.extensions)).toEqual(expectedNames); + expect(names(explicitlyResolved.extensions)).toEqual(expectedNames); + }); + + it("should resolve backslash-separated manifest globs consistently", async () => { + // Regression test for #9788. + const extensionDir = join(agentDir, "extensions", "backslash-glob-extension"); + mkdirSync(extensionDir, { recursive: true }); + writeFileSync(join(extensionDir, "main.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, "main.test.ts"), "export default function() {}"); + writeFileSync( + join(extensionDir, "package.json"), + JSON.stringify({ pi: { extensions: [".\\*.ts", "!.\\*.test.ts"] } }), + ); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const paths = (resources: ResolvedResource[]) => resources.map((resource) => resource.path); + + expect(paths(autoDiscovered.extensions)).toEqual([join(extensionDir, "main.ts")]); + expect(paths(explicitlyResolved.extensions)).toEqual([join(extensionDir, "main.ts")]); + }); + + it("should prefer the index entry when a manifest declares its own directory", async () => { + // Regression test for #9788. + const extensionDir = join(agentDir, "extensions", "self-directory-extension"); + mkdirSync(extensionDir, { recursive: true }); + writeFileSync(join(extensionDir, "index.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, "helper.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, "package.json"), JSON.stringify({ pi: { extensions: ["."] } })); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const paths = (resources: ResolvedResource[]) => resources.map((resource) => resource.path); + + expect(paths(autoDiscovered.extensions)).toEqual([join(extensionDir, "index.ts")]); + expect(paths(explicitlyResolved.extensions)).toEqual([join(extensionDir, "index.ts")]); + }); + + it("should prefer a declared child package main entry over scanning its contents", async () => { + // Regression test for #9788. + const extensionDir = join(agentDir, "extensions", "child-main-extension"); + const childDir = join(extensionDir, "child"); + mkdirSync(join(childDir, "dist"), { recursive: true }); + writeFileSync(join(childDir, "dist", "main.js"), "export default function() {}"); + writeFileSync(join(childDir, "helper.ts"), "export default function() {}"); + writeFileSync(join(childDir, "package.json"), JSON.stringify({ main: "./dist/main.js" })); + writeFileSync(join(extensionDir, "package.json"), JSON.stringify({ pi: { extensions: ["./child"] } })); + + const autoDiscovered = await packageManager.resolve(); + const explicitlyResolved = await packageManager.resolveExtensionSources([extensionDir]); + const paths = (resources: ResolvedResource[]) => resources.map((resource) => resource.path); + + expect(paths(autoDiscovered.extensions)).toEqual([join(childDir, "dist", "main.js")]); + expect(paths(explicitlyResolved.extensions)).toEqual([join(childDir, "dist", "main.js")]); + }); + + it("should fail when an extension package manifest contains malformed JSON", async () => { + // Regression test for #9788. + const extensionDir = join(agentDir, "extensions", "malformed-manifest-extension"); + mkdirSync(extensionDir, { recursive: true }); + writeFileSync(join(extensionDir, "index.ts"), "export default function() {}"); + writeFileSync(join(extensionDir, "package.json"), "{"); + + await expect(packageManager.resolve()).rejects.toThrow(SyntaxError); + }); + it("should resolve skill paths from settings", async () => { const skillDir = join(agentDir, "skills", "my-skill"); mkdirSync(skillDir, { recursive: true }); @@ -643,6 +797,25 @@ Content`, expect(result.extensions.some((r) => r.path === extPath && r.enabled)).toBe(true); }); + it("should reject cyclic local extension manifests", async () => { + const firstDir = join(tempDir, "first-cycle-package"); + const secondDir = join(tempDir, "second-cycle-package"); + mkdirSync(firstDir); + mkdirSync(secondDir); + writeFileSync( + join(firstDir, "package.json"), + JSON.stringify({ pi: { extensions: ["../second-cycle-package"] } }), + ); + writeFileSync( + join(secondDir, "package.json"), + JSON.stringify({ pi: { extensions: ["../first-cycle-package"] } }), + ); + + await expect(packageManager.resolveExtensionSources([firstDir])).rejects.toMatchObject({ + code: "EXTENSION_MANIFEST_CYCLE", + }); + }); + it("should handle directories with pi manifest", async () => { const pkgDir = join(tempDir, "my-package"); mkdirSync(pkgDir, { recursive: true });