From 50c83235904bb24e8d634cc25f7f761ec1324e98 Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Sat, 24 Jan 2026 00:35:19 +0100 Subject: [PATCH] feat(coding-agent): package deduplication and collision detection - Package deduplication: same package in global+project, project wins - Collision detection for skills, prompts, and themes with ResourceCollision type - PathMetadata tracking with parent directory lookup for file paths - Display improvements: section headers, sorted groups, accent colors for packages - pi list shows full paths below package names - Extension loader discovers files in directories without index.ts - In-memory SettingsManager properly tracks project settings fixes #645 --- packages/coding-agent/CHANGELOG.md | 2 + packages/coding-agent/docs/extensions.md | 5 + .../examples/sdk/08-prompt-templates.ts | 1 + .../examples/sdk/12-full-control.ts | 1 + .../coding-agent/src/core/agent-session.ts | 13 +- .../src/core/extensions/loader.ts | 4 + .../coding-agent/src/core/package-manager.ts | 594 ++++++++---------- .../coding-agent/src/core/prompt-templates.ts | 2 + .../coding-agent/src/core/resource-loader.ts | 136 +++- .../coding-agent/src/core/settings-manager.ts | 15 +- packages/coding-agent/src/core/skills.ts | 8 +- packages/coding-agent/src/index.ts | 10 +- packages/coding-agent/src/main.ts | 18 +- .../src/modes/interactive/interactive-mode.ts | 271 +++++++- .../coding-agent/test/package-manager.test.ts | 43 ++ packages/coding-agent/test/sdk-skills.test.ts | 2 + packages/coding-agent/test/utilities.ts | 1 + packages/mom/src/agent.ts | 1 + 18 files changed, 738 insertions(+), 389 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 05f33b1b..4dd863a1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -19,6 +19,8 @@ - Glob pattern support with minimatch in package filters, top-level settings arrays, and pi manifest (e.g., `"!funky.json"`, `"*.ts"`) ([#645](https://github.com/badlogic/pi-mono/issues/645)) - `/reload` command to reload extensions, skills, prompts, and themes ([#645](https://github.com/badlogic/pi-mono/issues/645)) - CLI flags for `--skill`, `--prompt-template`, `--theme`, `--no-prompt-templates`, and `--no-themes` ([#645](https://github.com/badlogic/pi-mono/issues/645)) +- Package deduplication: if same package appears in global and project settings, project wins ([#645](https://github.com/badlogic/pi-mono/issues/645)) +- Unified collision reporting with `ResourceDiagnostic` type for all resource types ([#645](https://github.com/badlogic/pi-mono/issues/645)) - Show provider alongside the model in the footer if multiple providers are available ### Changed diff --git a/packages/coding-agent/docs/extensions.md b/packages/coding-agent/docs/extensions.md index 9f060737..a71b5ea8 100644 --- a/packages/coding-agent/docs/extensions.md +++ b/packages/coding-agent/docs/extensions.md @@ -161,6 +161,11 @@ pi update # update all non-pinned packages - Glob patterns supported via minimatch (e.g., `"extensions": ["**/*.ts", "!**/deprecated/*"]`) - **Layered filtering:** User filters apply on top of manifest filters. If a manifest excludes 10 extensions and user adds one more exclusion, all 11 are excluded. +**Package deduplication:** If the same package appears in both global (`~/.pi/agent/settings.json`) and project (`.pi/settings.json`) settings, the project version wins and the global one is ignored. This prevents duplicate resource collisions when you have the same package installed at both scopes. Package identity is determined by: +- **npm packages:** Package name (e.g., `npm:foo` and `npm:foo@1.0.0` are the same identity) +- **git packages:** Repository URL without ref (e.g., `git:github.com/user/repo` and `git:github.com/user/repo@v1` are the same identity) +- **local paths:** Resolved absolute path + **Discovery rules:** 1. **Direct files:** `extensions/*.ts` or `*.js` → loaded directly diff --git a/packages/coding-agent/examples/sdk/08-prompt-templates.ts b/packages/coding-agent/examples/sdk/08-prompt-templates.ts index d417a801..76f916dd 100644 --- a/packages/coding-agent/examples/sdk/08-prompt-templates.ts +++ b/packages/coding-agent/examples/sdk/08-prompt-templates.ts @@ -16,6 +16,7 @@ const deployTemplate: PromptTemplate = { name: "deploy", description: "Deploy the application", source: "(custom)", + filePath: "", content: `# Deploy Instructions 1. Build: npm run build diff --git a/packages/coding-agent/examples/sdk/12-full-control.ts b/packages/coding-agent/examples/sdk/12-full-control.ts index c366b967..66823c83 100644 --- a/packages/coding-agent/examples/sdk/12-full-control.ts +++ b/packages/coding-agent/examples/sdk/12-full-control.ts @@ -53,6 +53,7 @@ const resourceLoader: ResourceLoader = { getSystemPrompt: () => `You are a minimal assistant. Available: read, bash. Be concise.`, getAppendSystemPrompt: () => [], + getPathMetadata: () => new Map(), reload: async () => {}, }; diff --git a/packages/coding-agent/src/core/agent-session.ts b/packages/coding-agent/src/core/agent-session.ts index 78a87e15..73e18a02 100644 --- a/packages/coding-agent/src/core/agent-session.ts +++ b/packages/coding-agent/src/core/agent-session.ts @@ -62,10 +62,10 @@ import { import type { BashExecutionMessage, CustomMessage } from "./messages.js"; import type { ModelRegistry } from "./model-registry.js"; import { expandPromptTemplate, type PromptTemplate } from "./prompt-templates.js"; -import type { ResourceLoader } from "./resource-loader.js"; +import type { ResourceDiagnostic, ResourceLoader } from "./resource-loader.js"; import type { BranchSummaryEntry, CompactionEntry, NewSessionOptions, SessionManager } from "./session-manager.js"; import type { SettingsManager } from "./settings-manager.js"; -import type { Skill, SkillWarning } from "./skills.js"; +import type { Skill } from "./skills.js"; import { buildSystemPrompt } from "./system-prompt.js"; import type { BashOperations } from "./tools/bash.js"; import { createAllTools } from "./tools/index.js"; @@ -1036,12 +1036,9 @@ export class AgentSession { return this._resourceLoader.getSkills().skills; } - /** Skill loading warnings captured by resource loader */ - get skillWarnings(): readonly SkillWarning[] { - return this._resourceLoader.getSkills().diagnostics.map((diagnostic) => ({ - skillPath: diagnostic.path ?? "", - message: diagnostic.message, - })); + /** Skill loading diagnostics (warnings, errors, collisions) */ + get skillWarnings(): readonly ResourceDiagnostic[] { + return this._resourceLoader.getSkills().diagnostics; } get resourceLoader(): ResourceLoader { diff --git a/packages/coding-agent/src/core/extensions/loader.ts b/packages/coding-agent/src/core/extensions/loader.ts index df10eda5..8c2f8721 100644 --- a/packages/coding-agent/src/core/extensions/loader.ts +++ b/packages/coding-agent/src/core/extensions/loader.ts @@ -487,11 +487,15 @@ export async function discoverAndLoadExtensions( for (const p of configuredPaths) { const resolved = resolvePath(p, cwd); 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; } addPaths([resolved]); diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 2534526e..1ee3db55 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -7,11 +7,18 @@ import { minimatch } from "minimatch"; import { CONFIG_DIR_NAME } from "../config.js"; import type { PackageSource, SettingsManager } from "./settings-manager.js"; +export interface PathMetadata { + source: string; + scope: SourceScope; + origin: "package" | "top-level"; +} + export interface ResolvedPaths { extensions: string[]; skills: string[]; prompts: string[]; themes: string[]; + metadata: Map; } export type MissingSourceAction = "install" | "skip" | "error"; @@ -35,6 +42,7 @@ export interface PackageManager { options?: { local?: boolean; temporary?: boolean }, ): Promise; setProgressCallback(callback: ProgressCallback | undefined): void; + getInstalledPath(source: string, scope: "global" | "project"): string | undefined; } interface PackageManagerOptions { @@ -80,6 +88,7 @@ interface ResourceAccumulator { skills: Set; prompts: Set; themes: Set; + metadata: Map; } interface PackageFilter { @@ -89,9 +98,10 @@ interface PackageFilter { themes?: string[]; } -// File type patterns for each resource type type ResourceType = "extensions" | "skills" | "prompts" | "themes"; +const RESOURCE_TYPES: ResourceType[] = ["extensions", "skills", "prompts", "themes"]; + const FILE_PATTERNS: Record = { extensions: /\.(ts|js)$/, skills: /\.md$/, @@ -99,23 +109,27 @@ const FILE_PATTERNS: Record = { themes: /\.json$/, }; -/** - * Check if a string contains glob pattern characters or is an exclusion. - */ function isPattern(s: string): boolean { return s.startsWith("!") || s.includes("*") || s.includes("?"); } -/** - * Check if any entry in the array is a pattern. - */ function hasPatterns(entries: string[]): boolean { return entries.some(isPattern); } -/** - * Recursively collect files from a directory matching a file pattern. - */ +function splitPatterns(entries: string[]): { plain: string[]; patterns: string[] } { + const plain: string[] = []; + const patterns: string[] = []; + for (const entry of entries) { + if (isPattern(entry)) { + patterns.push(entry); + } else { + plain.push(entry); + } + } + return { plain, patterns }; +} + function collectFiles(dir: string, filePattern: RegExp, skipNodeModules = true): string[] { const files: string[] = []; if (!existsSync(dir)) return files; @@ -153,10 +167,6 @@ function collectFiles(dir: string, filePattern: RegExp, skipNodeModules = true): return files; } -/** - * Collect skill entries from a directory. - * Skills can be directories (with SKILL.md) or direct .md files. - */ function collectSkillEntries(dir: string): string[] { const entries: string[] = []; if (!existsSync(dir)) return entries; @@ -182,7 +192,6 @@ function collectSkillEntries(dir: string): string[] { } if (isDir) { - // Skill directory - add if it has SKILL.md or recurse const skillMd = join(fullPath, "SKILL.md"); if (existsSync(skillMd)) { entries.push(fullPath); @@ -200,12 +209,6 @@ function collectSkillEntries(dir: string): string[] { return entries; } -/** - * Apply inclusion/exclusion patterns to filter paths. - * @param allPaths - All available paths to filter - * @param patterns - Array of patterns (prefix with ! for exclusion) - * @param baseDir - Base directory for relative pattern matching - */ function applyPatterns(allPaths: string[], patterns: string[], baseDir: string): string[] { const includes: string[] = []; const excludes: string[] = []; @@ -218,7 +221,6 @@ function applyPatterns(allPaths: string[], patterns: string[], baseDir: string): } } - // If only exclusions, start with all paths; otherwise filter to inclusions first let result: string[]; if (includes.length === 0) { result = [...allPaths]; @@ -227,13 +229,11 @@ function applyPatterns(allPaths: string[], patterns: string[], baseDir: string): const rel = relative(baseDir, filePath); const name = basename(filePath); return includes.some((pattern) => { - // Match against relative path, basename, or full path return minimatch(rel, pattern) || minimatch(name, pattern) || minimatch(filePath, pattern); }); }); } - // Apply exclusions if (excludes.length > 0) { result = result.filter((filePath) => { const rel = relative(baseDir, filePath); @@ -265,116 +265,85 @@ export class DefaultPackageManager implements PackageManager { this.progressCallback = callback; } + getInstalledPath(source: string, scope: "global" | "project"): string | undefined { + const parsed = this.parseSource(source); + if (parsed.type === "npm") { + const path = this.getNpmInstallPath(parsed, scope); + return existsSync(path) ? path : undefined; + } + if (parsed.type === "git") { + const path = this.getGitInstallPath(parsed, scope); + return existsSync(path) ? path : undefined; + } + if (parsed.type === "local") { + const path = this.resolvePath(parsed.path); + return existsSync(path) ? path : undefined; + } + return undefined; + } + private emitProgress(event: ProgressEvent): void { this.progressCallback?.(event); } + private async withProgress( + action: ProgressEvent["action"], + source: string, + message: string, + operation: () => Promise, + ): Promise { + this.emitProgress({ type: "start", action, source, message }); + try { + await operation(); + this.emitProgress({ type: "complete", action, source }); + } catch (error) { + const errorMessage = error instanceof Error ? error.message : String(error); + this.emitProgress({ type: "error", action, source, message: errorMessage }); + throw error; + } + } + async resolve(onMissing?: (source: string) => Promise): Promise { const accumulator = this.createAccumulator(); const globalSettings = this.settingsManager.getGlobalSettings(); const projectSettings = this.settingsManager.getProjectSettings(); - // Resolve packages (npm/git sources) - const packageSources: Array<{ pkg: PackageSource; scope: SourceScope }> = []; + // Collect all packages with scope + const allPackages: Array<{ pkg: PackageSource; scope: SourceScope }> = []; for (const pkg of globalSettings.packages ?? []) { - packageSources.push({ pkg, scope: "global" }); + allPackages.push({ pkg, scope: "global" }); } for (const pkg of projectSettings.packages ?? []) { - packageSources.push({ pkg, scope: "project" }); + allPackages.push({ pkg, scope: "project" }); } + + // Dedupe: project scope wins over global for same package identity + const packageSources = this.dedupePackages(allPackages); await this.resolvePackageSources(packageSources, accumulator, onMissing); - // Resolve local extensions - this.resolveLocalEntries( - [...(globalSettings.extensions ?? []), ...(projectSettings.extensions ?? [])], - "extensions", - accumulator.extensions, - ); - - // Resolve local skills - this.resolveLocalEntries( - [...(globalSettings.skills ?? []), ...(projectSettings.skills ?? [])], - "skills", - accumulator.skills, - ); - - // Resolve local prompts - this.resolveLocalEntries( - [...(globalSettings.prompts ?? []), ...(projectSettings.prompts ?? [])], - "prompts", - accumulator.prompts, - ); - - // Resolve local themes - this.resolveLocalEntries( - [...(globalSettings.themes ?? []), ...(projectSettings.themes ?? [])], - "themes", - accumulator.themes, - ); + for (const resourceType of RESOURCE_TYPES) { + const target = this.getTargetSet(accumulator, resourceType); + const globalEntries = (globalSettings[resourceType] ?? []) as string[]; + const projectEntries = (projectSettings[resourceType] ?? []) as string[]; + this.resolveLocalEntries( + globalEntries, + resourceType, + target, + { source: "local", scope: "global", origin: "top-level" }, + accumulator, + ); + this.resolveLocalEntries( + projectEntries, + resourceType, + target, + { source: "local", scope: "project", origin: "top-level" }, + accumulator, + ); + } return this.toResolvedPaths(accumulator); } - /** - * Resolve local entries with pattern support. - * If any entry contains patterns, enumerate files and apply filters. - * Otherwise, just resolve paths directly. - */ - private resolveLocalEntries(entries: string[], resourceType: ResourceType, target: Set): void { - if (entries.length === 0) return; - - if (!hasPatterns(entries)) { - // No patterns - resolve directly - for (const entry of entries) { - const resolved = this.resolvePath(entry); - if (existsSync(resolved)) { - this.addPath(target, resolved); - } - } - return; - } - - // Has patterns - need to enumerate and filter - const plainPaths: string[] = []; - const patterns: string[] = []; - - for (const entry of entries) { - if (isPattern(entry)) { - patterns.push(entry); - } else { - plainPaths.push(entry); - } - } - - // Collect all files from plain paths - const allFiles: string[] = []; - for (const p of plainPaths) { - const resolved = this.resolvePath(p); - if (!existsSync(resolved)) continue; - - try { - const stats = statSync(resolved); - if (stats.isFile()) { - allFiles.push(resolved); - } else if (stats.isDirectory()) { - if (resourceType === "skills") { - allFiles.push(...collectSkillEntries(resolved)); - } else { - allFiles.push(...collectFiles(resolved, FILE_PATTERNS[resourceType])); - } - } - } catch { - // Ignore errors - } - } - - // Apply patterns - const filtered = applyPatterns(allFiles, patterns, this.cwd); - for (const f of filtered) { - this.addPath(target, f); - } - } - async resolveExtensionSources( sources: string[], options?: { local?: boolean; temporary?: boolean }, @@ -389,47 +358,33 @@ export class DefaultPackageManager implements PackageManager { async install(source: string, options?: { local?: boolean }): Promise { const parsed = this.parseSource(source); const scope: SourceScope = options?.local ? "project" : "global"; - this.emitProgress({ type: "start", action: "install", source, message: `Installing ${source}...` }); - try { + await this.withProgress("install", source, `Installing ${source}...`, async () => { if (parsed.type === "npm") { await this.installNpm(parsed, scope, false); - this.emitProgress({ type: "complete", action: "install", source }); return; } if (parsed.type === "git") { await this.installGit(parsed, scope); - this.emitProgress({ type: "complete", action: "install", source }); return; } throw new Error(`Unsupported install source: ${source}`); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - this.emitProgress({ type: "error", action: "install", source, message }); - throw error; - } + }); } async remove(source: string, options?: { local?: boolean }): Promise { const parsed = this.parseSource(source); const scope: SourceScope = options?.local ? "project" : "global"; - this.emitProgress({ type: "start", action: "remove", source, message: `Removing ${source}...` }); - try { + await this.withProgress("remove", source, `Removing ${source}...`, async () => { if (parsed.type === "npm") { await this.uninstallNpm(parsed, scope); - this.emitProgress({ type: "complete", action: "remove", source }); return; } if (parsed.type === "git") { await this.removeGit(parsed, scope); - this.emitProgress({ type: "complete", action: "remove", source }); return; } throw new Error(`Unsupported remove source: ${source}`); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - this.emitProgress({ type: "error", action: "remove", source, message }); - throw error; - } + }); } async update(source?: string): Promise { @@ -453,28 +408,16 @@ export class DefaultPackageManager implements PackageManager { const parsed = this.parseSource(source); if (parsed.type === "npm") { if (parsed.pinned) return; - this.emitProgress({ type: "start", action: "update", source, message: `Updating ${source}...` }); - try { + await this.withProgress("update", source, `Updating ${source}...`, async () => { await this.installNpm(parsed, scope, false); - this.emitProgress({ type: "complete", action: "update", source }); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - this.emitProgress({ type: "error", action: "update", source, message }); - throw error; - } + }); return; } if (parsed.type === "git") { if (parsed.pinned) return; - this.emitProgress({ type: "start", action: "update", source, message: `Updating ${source}...` }); - try { + await this.withProgress("update", source, `Updating ${source}...`, async () => { await this.updateGit(parsed, scope); - this.emitProgress({ type: "complete", action: "update", source }); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); - this.emitProgress({ type: "error", action: "update", source, message }); - throw error; - } + }); return; } } @@ -488,9 +431,10 @@ export class DefaultPackageManager implements PackageManager { const sourceStr = typeof pkg === "string" ? pkg : pkg.source; const filter = typeof pkg === "object" ? pkg : undefined; const parsed = this.parseSource(sourceStr); + const metadata: PathMetadata = { source: sourceStr, scope, origin: "package" }; if (parsed.type === "local") { - this.resolveLocalExtensionSource(parsed, accumulator, filter); + this.resolveLocalExtensionSource(parsed, accumulator, filter, metadata); continue; } @@ -512,7 +456,7 @@ export class DefaultPackageManager implements PackageManager { const installed = await installMissing(); if (!installed) continue; } - this.collectPackageResources(installedPath, accumulator, filter); + this.collectPackageResources(installedPath, accumulator, filter, metadata); continue; } @@ -522,7 +466,7 @@ export class DefaultPackageManager implements PackageManager { const installed = await installMissing(); if (!installed) continue; } - this.collectPackageResources(installedPath, accumulator, filter); + this.collectPackageResources(installedPath, accumulator, filter, metadata); } } } @@ -530,7 +474,8 @@ export class DefaultPackageManager implements PackageManager { private resolveLocalExtensionSource( source: LocalSource, accumulator: ResourceAccumulator, - filter?: PackageFilter, + filter: PackageFilter | undefined, + metadata: PathMetadata, ): void { const resolved = this.resolvePath(source.path); if (!existsSync(resolved)) { @@ -540,13 +485,13 @@ export class DefaultPackageManager implements PackageManager { try { const stats = statSync(resolved); if (stats.isFile()) { - this.addPath(accumulator.extensions, resolved); + this.addPath(accumulator.extensions, resolved, metadata, accumulator); return; } if (stats.isDirectory()) { - const resources = this.collectPackageResources(resolved, accumulator, filter); + const resources = this.collectPackageResources(resolved, accumulator, filter, metadata); if (!resources) { - this.addPath(accumulator.extensions, resolved); + this.addPath(accumulator.extensions, resolved, metadata, accumulator); } } } catch { @@ -577,7 +522,6 @@ export class DefaultPackageManager implements PackageManager { }; } - // Accept git: prefix or raw URLs (https://github.com/..., github.com/...) if (source.startsWith("git:") || this.looksLikeGitUrl(source)) { const repoSpec = source.startsWith("git:") ? source.slice("git:".length).trim() : source; const [repo, ref] = repoSpec.split("@"); @@ -599,12 +543,54 @@ export class DefaultPackageManager implements PackageManager { } private looksLikeGitUrl(source: string): boolean { - // Match URLs like https://github.com/..., github.com/..., gitlab.com/... const gitHosts = ["github.com", "gitlab.com", "bitbucket.org", "codeberg.org"]; const normalized = source.replace(/^https?:\/\//, ""); return gitHosts.some((host) => normalized.startsWith(`${host}/`)); } + /** + * Get a unique identity for a package, ignoring version/ref. + * Used to detect when the same package is in both global and project settings. + */ + private getPackageIdentity(source: string): string { + const parsed = this.parseSource(source); + if (parsed.type === "npm") { + return `npm:${parsed.name}`; + } + if (parsed.type === "git") { + return `git:${parsed.repo}`; + } + // For local paths, use the absolute resolved path + return `local:${this.resolvePath(parsed.path)}`; + } + + /** + * Dedupe packages: if same package identity appears in both global and project, + * keep only the project one (project wins). + */ + private dedupePackages( + packages: Array<{ pkg: PackageSource; scope: SourceScope }>, + ): Array<{ pkg: PackageSource; scope: SourceScope }> { + const seen = new Map(); + + for (const entry of packages) { + const sourceStr = typeof entry.pkg === "string" ? entry.pkg : entry.pkg.source; + const identity = this.getPackageIdentity(sourceStr); + + const existing = seen.get(identity); + if (!existing) { + seen.set(identity, entry); + } else if (entry.scope === "project" && existing.scope === "global") { + // Project wins over global + seen.set(identity, entry); + } + // If existing is project and new is global, keep existing (project) + // If both are same scope, keep first one + } + + return Array.from(seen.values()); + } + private parseNpmSpec(spec: string): { name: string; version?: string } { const match = spec.match(/^(@?[^@]+(?:\/[^@]+)?)(?:@(.+))?$/); if (!match) { @@ -648,7 +634,6 @@ export class DefaultPackageManager implements PackageManager { if (source.ref) { await this.runCommand("git", ["checkout", source.ref], { cwd: targetDir }); } - // Install npm dependencies if package.json exists const packageJsonPath = join(targetDir, "package.json"); if (existsSync(packageJsonPath)) { await this.runCommand("npm", ["install"], { cwd: targetDir }); @@ -662,7 +647,6 @@ export class DefaultPackageManager implements PackageManager { return; } await this.runCommand("git", ["pull"], { cwd: targetDir }); - // Reinstall npm dependencies if package.json exists (in case deps changed) const packageJsonPath = join(targetDir, "package.json"); if (existsSync(packageJsonPath)) { await this.runCommand("npm", ["install"], { cwd: targetDir }); @@ -762,200 +746,103 @@ export class DefaultPackageManager implements PackageManager { private collectPackageResources( packageRoot: string, accumulator: ResourceAccumulator, - filter?: PackageFilter, + filter: PackageFilter | undefined, + metadata: PathMetadata, ): boolean { - // If filter is provided, use it to selectively load resources if (filter) { - // Empty array means "load none", undefined means "load all" - if (filter.extensions !== undefined) { - this.applyPackageFilter(packageRoot, filter.extensions, "extensions", accumulator.extensions); - } else { - this.collectDefaultExtensions(packageRoot, accumulator); + for (const resourceType of RESOURCE_TYPES) { + const patterns = filter[resourceType as keyof PackageFilter]; + const target = this.getTargetSet(accumulator, resourceType); + if (patterns !== undefined) { + this.applyPackageFilter(packageRoot, patterns, resourceType, target, metadata, accumulator); + } else { + this.collectDefaultResources(packageRoot, resourceType, target, metadata, accumulator); + } } - - if (filter.skills !== undefined) { - this.applyPackageFilter(packageRoot, filter.skills, "skills", accumulator.skills); - } else { - this.collectDefaultSkills(packageRoot, accumulator); - } - - if (filter.prompts !== undefined) { - this.applyPackageFilter(packageRoot, filter.prompts, "prompts", accumulator.prompts); - } else { - this.collectDefaultPrompts(packageRoot, accumulator); - } - - if (filter.themes !== undefined) { - this.applyPackageFilter(packageRoot, filter.themes, "themes", accumulator.themes); - } else { - this.collectDefaultThemes(packageRoot, accumulator); - } - return true; } - // No filter: load everything based on manifest or directory structure const manifest = this.readPiManifest(packageRoot); if (manifest) { - this.addManifestEntries(manifest.extensions, packageRoot, "extensions", accumulator.extensions); - this.addManifestEntries(manifest.skills, packageRoot, "skills", accumulator.skills); - this.addManifestEntries(manifest.prompts, packageRoot, "prompts", accumulator.prompts); - this.addManifestEntries(manifest.themes, packageRoot, "themes", accumulator.themes); + for (const resourceType of RESOURCE_TYPES) { + const entries = manifest[resourceType as keyof PiManifest]; + this.addManifestEntries( + entries, + packageRoot, + resourceType, + this.getTargetSet(accumulator, resourceType), + metadata, + accumulator, + ); + } return true; } - const extensionsDir = join(packageRoot, "extensions"); - const skillsDir = join(packageRoot, "skills"); - const promptsDir = join(packageRoot, "prompts"); - const themesDir = join(packageRoot, "themes"); - - const hasAnyDir = - existsSync(extensionsDir) || existsSync(skillsDir) || existsSync(promptsDir) || existsSync(themesDir); - if (!hasAnyDir) { - return false; + let hasAnyDir = false; + for (const resourceType of RESOURCE_TYPES) { + const dir = join(packageRoot, resourceType); + if (existsSync(dir)) { + this.addPath(this.getTargetSet(accumulator, resourceType), dir, metadata, accumulator); + hasAnyDir = true; + } } - - if (existsSync(extensionsDir)) { - this.addPath(accumulator.extensions, extensionsDir); - } - if (existsSync(skillsDir)) { - this.addPath(accumulator.skills, skillsDir); - } - if (existsSync(promptsDir)) { - this.addPath(accumulator.prompts, promptsDir); - } - if (existsSync(themesDir)) { - this.addPath(accumulator.themes, themesDir); - } - return true; + return hasAnyDir; } - private collectDefaultExtensions(packageRoot: string, accumulator: ResourceAccumulator): void { + private collectDefaultResources( + packageRoot: string, + resourceType: ResourceType, + target: Set, + metadata: PathMetadata, + accumulator: ResourceAccumulator, + ): void { const manifest = this.readPiManifest(packageRoot); - if (manifest?.extensions) { - this.addManifestEntries(manifest.extensions, packageRoot, "extensions", accumulator.extensions); + const entries = manifest?.[resourceType as keyof PiManifest]; + if (entries) { + this.addManifestEntries(entries, packageRoot, resourceType, target, metadata, accumulator); return; } - const extensionsDir = join(packageRoot, "extensions"); - if (existsSync(extensionsDir)) { - this.addPath(accumulator.extensions, extensionsDir); + const dir = join(packageRoot, resourceType); + if (existsSync(dir)) { + this.addPath(target, dir, metadata, accumulator); } } - private collectDefaultSkills(packageRoot: string, accumulator: ResourceAccumulator): void { - const manifest = this.readPiManifest(packageRoot); - if (manifest?.skills) { - this.addManifestEntries(manifest.skills, packageRoot, "skills", accumulator.skills); - return; - } - const skillsDir = join(packageRoot, "skills"); - if (existsSync(skillsDir)) { - this.addPath(accumulator.skills, skillsDir); - } - } - - private collectDefaultPrompts(packageRoot: string, accumulator: ResourceAccumulator): void { - const manifest = this.readPiManifest(packageRoot); - if (manifest?.prompts) { - this.addManifestEntries(manifest.prompts, packageRoot, "prompts", accumulator.prompts); - return; - } - const promptsDir = join(packageRoot, "prompts"); - if (existsSync(promptsDir)) { - this.addPath(accumulator.prompts, promptsDir); - } - } - - private collectDefaultThemes(packageRoot: string, accumulator: ResourceAccumulator): void { - const manifest = this.readPiManifest(packageRoot); - if (manifest?.themes) { - this.addManifestEntries(manifest.themes, packageRoot, "themes", accumulator.themes); - return; - } - const themesDir = join(packageRoot, "themes"); - if (existsSync(themesDir)) { - this.addPath(accumulator.themes, themesDir); - } - } - - /** - * Apply user filter patterns on top of what the manifest provides. - * Manifest patterns are applied first, then user patterns narrow down further. - */ private applyPackageFilter( packageRoot: string, userPatterns: string[], resourceType: ResourceType, target: Set, + metadata: PathMetadata, + accumulator: ResourceAccumulator, ): void { if (userPatterns.length === 0) { - // Empty array = load none return; } - // First get what manifest provides (with manifest patterns already applied) const manifestFiles = this.collectManifestFilteredFiles(packageRoot, resourceType); - - // Then apply user patterns on top const filtered = applyPatterns(manifestFiles, userPatterns, packageRoot); for (const f of filtered) { - this.addPath(target, f); + this.addPath(target, f, metadata, accumulator); } } - /** - * Collect files that the manifest provides, with manifest patterns applied. - * This is what the package makes available before user filtering. - */ private collectManifestFilteredFiles(packageRoot: string, resourceType: ResourceType): string[] { const manifest = this.readPiManifest(packageRoot); - - if (manifest) { - const manifestEntries = manifest[resourceType]; - if (manifestEntries && manifestEntries.length > 0) { - // Enumerate all files from non-pattern entries - const allFiles: string[] = []; - for (const entry of manifestEntries) { - if (isPattern(entry)) continue; - - const resolved = resolve(packageRoot, entry); - if (!existsSync(resolved)) continue; - - try { - const stats = statSync(resolved); - if (stats.isFile()) { - allFiles.push(resolved); - } else if (stats.isDirectory()) { - if (resourceType === "skills") { - allFiles.push(...collectSkillEntries(resolved)); - } else { - allFiles.push(...collectFiles(resolved, FILE_PATTERNS[resourceType])); - } - } - } catch { - // Ignore errors - } - } - - // Apply manifest patterns (if any) - const manifestPatterns = manifestEntries.filter(isPattern); - if (manifestPatterns.length > 0) { - return applyPatterns(allFiles, manifestPatterns, packageRoot); - } - return allFiles; - } + const entries = manifest?.[resourceType as keyof PiManifest]; + if (entries && entries.length > 0) { + const allFiles = this.collectFilesFromManifestEntries(entries, packageRoot, resourceType); + const manifestPatterns = entries.filter(isPattern); + return manifestPatterns.length > 0 ? applyPatterns(allFiles, manifestPatterns, packageRoot) : allFiles; } - // Fall back to convention-based directories const conventionDir = join(packageRoot, resourceType); if (!existsSync(conventionDir)) { return []; } - - if (resourceType === "skills") { - return collectSkillEntries(conventionDir); - } - return collectFiles(conventionDir, FILE_PATTERNS[resourceType]); + return resourceType === "skills" + ? collectSkillEntries(conventionDir) + : collectFiles(conventionDir, FILE_PATTERNS[resourceType]); } private readPiManifest(packageRoot: string): PiManifest | null { @@ -978,48 +865,75 @@ export class DefaultPackageManager implements PackageManager { root: string, resourceType: ResourceType, target: Set, + metadata: PathMetadata, + accumulator: ResourceAccumulator, ): void { if (!entries) return; if (!hasPatterns(entries)) { - // No patterns - resolve directly for (const entry of entries) { const resolved = resolve(root, entry); - this.addPath(target, resolved); + this.addPath(target, resolved, metadata, accumulator); } return; } - // Has patterns - enumerate and filter - const allFiles = this.collectAllManifestFiles(entries, root, resourceType); + const allFiles = this.collectFilesFromManifestEntries(entries, root, resourceType); const patterns = entries.filter(isPattern); const filtered = applyPatterns(allFiles, patterns, root); for (const f of filtered) { - this.addPath(target, f); + this.addPath(target, f, metadata, accumulator); } } - /** - * Collect files from manifest entries for pattern matching. - * Plain paths are resolved and enumerated; pattern strings are skipped (used for filtering). - */ - private collectAllManifestFiles(entries: string[], root: string, resourceType: ResourceType): string[] { - const files: string[] = []; - for (const entry of entries) { - if (isPattern(entry)) continue; + private collectFilesFromManifestEntries(entries: string[], root: string, resourceType: ResourceType): string[] { + const plain = entries.filter((entry) => !isPattern(entry)); + const resolved = plain.map((entry) => resolve(root, entry)); + return this.collectFilesFromPaths(resolved, resourceType); + } - const resolved = resolve(root, entry); - if (!existsSync(resolved)) continue; + private resolveLocalEntries( + entries: string[], + resourceType: ResourceType, + target: Set, + metadata: PathMetadata, + accumulator: ResourceAccumulator, + ): void { + if (entries.length === 0) return; + + if (!hasPatterns(entries)) { + for (const entry of entries) { + const resolved = this.resolvePath(entry); + if (existsSync(resolved)) { + this.addPath(target, resolved, metadata, accumulator); + } + } + return; + } + + const { plain, patterns } = splitPatterns(entries); + const resolvedPlain = plain.map((p) => this.resolvePath(p)); + const allFiles = this.collectFilesFromPaths(resolvedPlain, resourceType); + const filtered = applyPatterns(allFiles, patterns, this.cwd); + for (const f of filtered) { + this.addPath(target, f, metadata, accumulator); + } + } + + private collectFilesFromPaths(paths: string[], resourceType: ResourceType): string[] { + const files: string[] = []; + for (const p of paths) { + if (!existsSync(p)) continue; try { - const stats = statSync(resolved); + const stats = statSync(p); if (stats.isFile()) { - files.push(resolved); + files.push(p); } else if (stats.isDirectory()) { if (resourceType === "skills") { - files.push(...collectSkillEntries(resolved)); + files.push(...collectSkillEntries(p)); } else { - files.push(...collectFiles(resolved, FILE_PATTERNS[resourceType])); + files.push(...collectFiles(p, FILE_PATTERNS[resourceType])); } } } catch { @@ -1029,9 +943,27 @@ export class DefaultPackageManager implements PackageManager { return files; } - private addPath(set: Set, value: string): void { + private getTargetSet(accumulator: ResourceAccumulator, resourceType: ResourceType): Set { + switch (resourceType) { + case "extensions": + return accumulator.extensions; + case "skills": + return accumulator.skills; + case "prompts": + return accumulator.prompts; + case "themes": + return accumulator.themes; + default: + throw new Error(`Unknown resource type: ${resourceType}`); + } + } + + private addPath(set: Set, value: string, metadata?: PathMetadata, accumulator?: ResourceAccumulator): void { if (!value) return; set.add(value); + if (metadata && accumulator && !accumulator.metadata.has(value)) { + accumulator.metadata.set(value, metadata); + } } private createAccumulator(): ResourceAccumulator { @@ -1040,6 +972,7 @@ export class DefaultPackageManager implements PackageManager { skills: new Set(), prompts: new Set(), themes: new Set(), + metadata: new Map(), }; } @@ -1049,6 +982,7 @@ export class DefaultPackageManager implements PackageManager { skills: Array.from(accumulator.skills), prompts: Array.from(accumulator.prompts), themes: Array.from(accumulator.themes), + metadata: accumulator.metadata, }; } diff --git a/packages/coding-agent/src/core/prompt-templates.ts b/packages/coding-agent/src/core/prompt-templates.ts index 51e5338d..0edd3288 100644 --- a/packages/coding-agent/src/core/prompt-templates.ts +++ b/packages/coding-agent/src/core/prompt-templates.ts @@ -12,6 +12,7 @@ export interface PromptTemplate { description: string; content: string; source: string; // e.g., "(user)", "(project)", "(custom:my-dir)" + filePath: string; // Absolute path to the template file } /** @@ -124,6 +125,7 @@ function loadTemplateFromFile(filePath: string, sourceLabel: string): PromptTemp description, content: body, source: sourceLabel, + filePath, }; } catch { return null; diff --git a/packages/coding-agent/src/core/resource-loader.ts b/packages/coding-agent/src/core/resource-loader.ts index 346dc696..54a18ba5 100644 --- a/packages/coding-agent/src/core/resource-loader.ts +++ b/packages/coding-agent/src/core/resource-loader.ts @@ -12,17 +12,27 @@ import { loadExtensions, } from "./extensions/loader.js"; import type { Extension, ExtensionFactory, ExtensionRuntime, LoadExtensionsResult } from "./extensions/types.js"; -import { DefaultPackageManager } from "./package-manager.js"; +import { DefaultPackageManager, type PathMetadata } from "./package-manager.js"; import type { PromptTemplate } from "./prompt-templates.js"; import { loadPromptTemplates } from "./prompt-templates.js"; import { SettingsManager } from "./settings-manager.js"; import type { Skill, SkillWarning } from "./skills.js"; import { loadSkills } from "./skills.js"; +export interface ResourceCollision { + resourceType: "extension" | "skill" | "prompt" | "theme"; + name: string; // skill name, command/tool/flag name, prompt name, theme name + winnerPath: string; + loserPath: string; + winnerSource?: string; // e.g., "npm:foo", "git:...", "local" + loserSource?: string; +} + export interface ResourceDiagnostic { - type: "warning" | "error"; + type: "warning" | "error" | "collision"; message: string; path?: string; + collision?: ResourceCollision; } export interface ResourceLoader { @@ -33,6 +43,7 @@ export interface ResourceLoader { getAgentsFiles(): { agentsFiles: Array<{ path: string; content: string }> }; getSystemPrompt(): string | undefined; getAppendSystemPrompt(): string[]; + getPathMetadata(): Map; reload(): Promise; } @@ -192,6 +203,7 @@ export class DefaultResourceLoader implements ResourceLoader { private agentsFiles: Array<{ path: string; content: string }>; private systemPrompt?: string; private appendSystemPrompt: string[]; + private pathMetadata: Map; constructor(options: DefaultResourceLoaderOptions) { this.cwd = options.cwd ?? process.cwd(); @@ -231,6 +243,7 @@ export class DefaultResourceLoader implements ResourceLoader { this.themeDiagnostics = []; this.agentsFiles = []; this.appendSystemPrompt = []; + this.pathMetadata = new Map(); } getExtensions(): LoadExtensionsResult { @@ -261,12 +274,30 @@ export class DefaultResourceLoader implements ResourceLoader { return this.appendSystemPrompt; } + getPathMetadata(): Map { + return this.pathMetadata; + } + async reload(): Promise { const resolvedPaths = await this.packageManager.resolve(); const cliExtensionPaths = await this.packageManager.resolveExtensionSources(this.additionalExtensionPaths, { temporary: true, }); + // Store metadata from resolved paths + this.pathMetadata = new Map(resolvedPaths.metadata); + // Add CLI paths metadata + for (const p of cliExtensionPaths.extensions) { + if (!this.pathMetadata.has(p)) { + this.pathMetadata.set(p, { source: "cli", scope: "temporary", origin: "top-level" }); + } + } + for (const p of cliExtensionPaths.skills) { + if (!this.pathMetadata.has(p)) { + this.pathMetadata.set(p, { source: "cli", scope: "temporary", origin: "top-level" }); + } + } + const extensionPaths = this.noExtensions ? cliExtensionPaths.extensions : this.mergePaths(resolvedPaths.extensions, cliExtensionPaths.extensions); @@ -306,7 +337,7 @@ export class DefaultResourceLoader implements ResourceLoader { agentDir: this.agentDir, skillPaths, }); - skillsResult = { skills: result.skills, diagnostics: this.toDiagnostics(result.warnings) }; + skillsResult = { skills: result.skills, diagnostics: this.skillWarningsToDiagnostics(result.warnings) }; } const resolvedSkills = this.skillsOverride ? this.skillsOverride(skillsResult) : skillsResult; this.skills = resolvedSkills.skills; @@ -323,14 +354,12 @@ export class DefaultResourceLoader implements ResourceLoader { if (this.noPromptTemplates && promptPaths.length === 0) { promptsResult = { prompts: [], diagnostics: [] }; } else { - promptsResult = { - prompts: loadPromptTemplates({ - cwd: this.cwd, - agentDir: this.agentDir, - promptPaths, - }), - diagnostics: [], - }; + const allPrompts = loadPromptTemplates({ + cwd: this.cwd, + agentDir: this.agentDir, + promptPaths, + }); + promptsResult = this.dedupePrompts(allPrompts); } const resolvedPrompts = this.promptsOverride ? this.promptsOverride(promptsResult) : promptsResult; this.prompts = resolvedPrompts.prompts; @@ -344,7 +373,9 @@ export class DefaultResourceLoader implements ResourceLoader { if (this.noThemes && themePaths.length === 0) { themesResult = { themes: [], diagnostics: [] }; } else { - themesResult = this.loadThemes(themePaths); + const loaded = this.loadThemes(themePaths); + const deduped = this.dedupeThemes(loaded.themes); + themesResult = { themes: deduped.themes, diagnostics: [...loaded.diagnostics, ...deduped.diagnostics] }; } const resolvedThemes = this.themesOverride ? this.themesOverride(themesResult) : themesResult; this.themes = resolvedThemes.themes; @@ -489,12 +520,81 @@ export class DefaultResourceLoader implements ResourceLoader { return { extensions, errors }; } - private toDiagnostics(warnings: SkillWarning[]): ResourceDiagnostic[] { - return warnings.map((warning) => ({ - type: "warning", - message: warning.message, - path: warning.skillPath, - })); + private skillWarningsToDiagnostics(warnings: SkillWarning[]): ResourceDiagnostic[] { + return warnings.map((w) => { + // If it's a name collision, create proper collision structure + if (w.collisionName && w.collisionWinner) { + return { + type: "collision" as const, + message: w.message, + path: w.skillPath, + collision: { + resourceType: "skill" as const, + name: w.collisionName, + winnerPath: w.collisionWinner, + loserPath: w.skillPath, + }, + }; + } + return { + type: "warning" as const, + message: w.message, + path: w.skillPath, + }; + }); + } + + private dedupePrompts(prompts: PromptTemplate[]): { prompts: PromptTemplate[]; diagnostics: ResourceDiagnostic[] } { + const seen = new Map(); + const diagnostics: ResourceDiagnostic[] = []; + + for (const prompt of prompts) { + const existing = seen.get(prompt.name); + if (existing) { + diagnostics.push({ + type: "collision", + message: `name "/${prompt.name}" collision`, + path: prompt.filePath, + collision: { + resourceType: "prompt", + name: prompt.name, + winnerPath: existing.filePath, + loserPath: prompt.filePath, + }, + }); + } else { + seen.set(prompt.name, prompt); + } + } + + return { prompts: Array.from(seen.values()), diagnostics }; + } + + private dedupeThemes(themes: Theme[]): { themes: Theme[]; diagnostics: ResourceDiagnostic[] } { + const seen = new Map(); + const diagnostics: ResourceDiagnostic[] = []; + + for (const t of themes) { + const name = t.name ?? "unnamed"; + const existing = seen.get(name); + if (existing) { + diagnostics.push({ + type: "collision", + message: `name "${name}" collision`, + path: t.sourcePath, + collision: { + resourceType: "theme", + name, + winnerPath: existing.sourcePath ?? "", + loserPath: t.sourcePath ?? "", + }, + }); + } else { + seen.set(name, t); + } + } + + return { themes: Array.from(seen.values()), diagnostics }; } private discoverSystemPromptFile(): string | undefined { diff --git a/packages/coding-agent/src/core/settings-manager.ts b/packages/coding-agent/src/core/settings-manager.ts index 523ee0ed..acdf8e8d 100644 --- a/packages/coding-agent/src/core/settings-manager.ts +++ b/packages/coding-agent/src/core/settings-manager.ts @@ -120,6 +120,7 @@ export class SettingsManager { private settingsPath: string | null; private projectSettingsPath: string | null; private globalSettings: Settings; + private inMemoryProjectSettings: Settings; // For in-memory mode private settings: Settings; private persist: boolean; @@ -133,6 +134,7 @@ export class SettingsManager { this.projectSettingsPath = projectSettingsPath; this.persist = persist; this.globalSettings = initialSettings; + this.inMemoryProjectSettings = {}; const projectSettings = this.loadProjectSettings(); this.settings = deepMergeSettings(this.globalSettings, projectSettings); } @@ -227,6 +229,11 @@ export class SettingsManager { } private loadProjectSettings(): Settings { + // In-memory mode: return stored in-memory project settings + if (!this.persist) { + return structuredClone(this.inMemoryProjectSettings); + } + if (!this.projectSettingsPath || !existsSync(this.projectSettingsPath)) { return {}; } @@ -281,7 +288,13 @@ export class SettingsManager { } private saveProjectSettings(settings: Settings): void { - if (!this.persist || !this.projectSettingsPath) { + // In-memory mode: store in memory + if (!this.persist) { + this.inMemoryProjectSettings = structuredClone(settings); + return; + } + + if (!this.projectSettingsPath) { return; } try { diff --git a/packages/coding-agent/src/core/skills.ts b/packages/coding-agent/src/core/skills.ts index 489e5848..eb5d1b62 100644 --- a/packages/coding-agent/src/core/skills.ts +++ b/packages/coding-agent/src/core/skills.ts @@ -40,6 +40,10 @@ export interface Skill { export interface SkillWarning { skillPath: string; message: string; + /** For name collisions, the name that collided */ + collisionName?: string; + /** For name collisions, the path of the skill that was loaded (winner) */ + collisionWinner?: string; } export interface LoadSkillsResult { @@ -338,7 +342,9 @@ export function loadSkills(options: LoadSkillsOptions = {}): LoadSkillsResult { if (existing) { collisionWarnings.push({ skillPath: skill.filePath, - message: `name collision: "${skill.name}" already loaded from ${existing.filePath}, skipping this one`, + message: `name "${skill.name}" collision`, + collisionName: skill.name, + collisionWinner: existing.filePath, }); } else { skillMap.set(skill.name, skill); diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index b278979f..3e8296a2 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -118,9 +118,15 @@ export { export type { ReadonlyFooterDataProvider } from "./core/footer-data-provider.js"; export { convertToLlm } from "./core/messages.js"; export { ModelRegistry } from "./core/model-registry.js"; -export type { PackageManager, ProgressCallback, ProgressEvent } from "./core/package-manager.js"; +export type { + PackageManager, + PathMetadata, + ProgressCallback, + ProgressEvent, + ResolvedPaths, +} from "./core/package-manager.js"; export { DefaultPackageManager } from "./core/package-manager.js"; -export type { ResourceDiagnostic, ResourceLoader } from "./core/resource-loader.js"; +export type { ResourceCollision, ResourceDiagnostic, ResourceLoader } from "./core/resource-loader.js"; export { DefaultResourceLoader } from "./core/resource-loader.js"; // SDK for programmatic usage export { diff --git a/packages/coding-agent/src/main.ts b/packages/coding-agent/src/main.ts index 8c4d7536..695c4cf0 100644 --- a/packages/coding-agent/src/main.ts +++ b/packages/coding-agent/src/main.ts @@ -190,11 +190,22 @@ async function handlePackageCommand(args: string[]): Promise { return true; } + const formatPackage = (pkg: (typeof globalPackages)[number], scope: "global" | "project") => { + const source = typeof pkg === "string" ? pkg : pkg.source; + const filtered = typeof pkg === "object"; + const display = filtered ? `${source} (filtered)` : source; + console.log(` ${display}`); + // Show resolved path + const path = packageManager.getInstalledPath(source, scope); + if (path) { + console.log(chalk.dim(` ${path}`)); + } + }; + if (globalPackages.length > 0) { console.log(chalk.bold("Global packages:")); for (const pkg of globalPackages) { - const display = typeof pkg === "string" ? pkg : `${pkg.source} (filtered)`; - console.log(` ${display}`); + formatPackage(pkg, "global"); } } @@ -202,8 +213,7 @@ async function handlePackageCommand(args: string[]): Promise { if (globalPackages.length > 0) console.log(); console.log(chalk.bold("Project packages:")); for (const pkg of projectPackages) { - const display = typeof pkg === "string" ? pkg : `${pkg.source} (filtered)`; - console.log(` ${display}`); + formatPackage(pkg, "project"); } } diff --git a/packages/coding-agent/src/modes/interactive/interactive-mode.ts b/packages/coding-agent/src/modes/interactive/interactive-mode.ts index 48674319..32f23e84 100644 --- a/packages/coding-agent/src/modes/interactive/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive/interactive-mode.ts @@ -65,6 +65,7 @@ import { FooterDataProvider, type ReadonlyFooterDataProvider } from "../../core/ import { type AppAction, KeybindingsManager } from "../../core/keybindings.js"; import { createCompactionSummaryMessage } from "../../core/messages.js"; import { resolveModelScope } from "../../core/model-resolver.js"; +import type { ResourceDiagnostic } from "../../core/resource-loader.js"; import { type SessionContext, SessionManager } from "../../core/session-manager.js"; import type { TruncationResult } from "../../core/tools/truncate.js"; import { getChangelogPath, getNewEntries, parseChangelog } from "../../utils/changelog.js"; @@ -628,18 +629,6 @@ export class InteractiveMode { const home = os.homedir(); let result = p; - // Shorten temp npm paths: /tmp/.../npm/.../node_modules/pkg/... -> npm:pkg/... (temp) - const npmTempMatch = result.match(/pi-extensions\/npm\/[^/]+\/node_modules\/([^/]+)(.*)/); - if (npmTempMatch) { - return `npm:${npmTempMatch[1]}${npmTempMatch[2]} (temp)`; - } - - // Shorten temp git paths: /tmp/.../git-host/hash/path/... -> git:host/path/... (temp) - const gitTempMatch = result.match(/pi-extensions\/git-([^/]+)\/[^/]+\/(.*)/); - if (gitTempMatch) { - return `git:${gitTempMatch[1]}/${gitTempMatch[2]} (temp)`; - } - // Replace home directory with ~ if (result.startsWith(home)) { result = `~${result.slice(home.length)}`; @@ -648,46 +637,278 @@ export class InteractiveMode { return result; } + /** + * Get a short path relative to the package root for display. + */ + private getShortPath(fullPath: string, source: string): string { + // For npm packages, show path relative to node_modules/pkg/ + const npmMatch = fullPath.match(/node_modules\/(@?[^/]+(?:\/[^/]+)?)\/(.*)/); + if (npmMatch && source.startsWith("npm:")) { + return npmMatch[2]; + } + + // For git packages, show path relative to repo root + const gitMatch = fullPath.match(/git\/[^/]+\/[^/]+\/(.*)/); + if (gitMatch && source.startsWith("git:")) { + return gitMatch[1]; + } + + // For local/auto, just use formatDisplayPath + return this.formatDisplayPath(fullPath); + } + + /** + * Group paths by source and scope using metadata. + * Returns sorted: local first, then global packages, then project packages. + */ + private groupPathsBySource( + paths: string[], + metadata: Map, + ): Map { + const groups = new Map(); + + for (const p of paths) { + const meta = this.findMetadata(p, metadata); + const source = meta?.source ?? "local"; + const scope = meta?.scope ?? "project"; + + if (!groups.has(source)) { + groups.set(source, { scope, paths: [] }); + } + groups.get(source)!.paths.push(p); + } + + // Sort: local first, then global packages, then project packages + const sorted = new Map(); + const entries = Array.from(groups.entries()); + + // Local entries first + for (const [source, data] of entries) { + if (source === "local") sorted.set(source, data); + } + // Global packages + for (const [source, data] of entries) { + if (source !== "local" && data.scope === "global") sorted.set(source, data); + } + // Project packages + for (const [source, data] of entries) { + if (source !== "local" && data.scope === "project") sorted.set(source, data); + } + + return sorted; + } + + /** + * Format grouped paths for display with colors. + */ + private formatGroupedPaths( + groups: Map, + formatPath: (p: string, source: string) => string, + ): string { + const lines: string[] = []; + + for (const [source, { scope, paths }] of groups) { + const scopeLabel = scope === "global" ? "global" : scope === "project" ? "project" : ""; + // Source name in accent, scope in muted + const sourceColor = source === "local" ? "muted" : "accent"; + const header = scopeLabel + ? `${theme.fg(sourceColor, source)} ${theme.fg("dim", `(${scopeLabel})`)}` + : theme.fg(sourceColor, source); + lines.push(` ${header}`); + for (const p of paths) { + lines.push(theme.fg("dim", ` ${formatPath(p, source)}`)); + } + } + + return lines.join("\n"); + } + + /** + * Find metadata for a path, checking parent directories if exact match fails. + * Package manager stores metadata for directories, but we display file paths. + */ + private findMetadata( + p: string, + metadata: Map, + ): { source: string; scope: string; origin: string } | undefined { + // Try exact match first + const exact = metadata.get(p); + if (exact) return exact; + + // Try parent directories (package manager stores directory paths) + let current = p; + while (current.includes("/")) { + current = current.substring(0, current.lastIndexOf("/")); + const parent = metadata.get(current); + if (parent) return parent; + } + + return undefined; + } + + /** + * Format a path with its source/scope info from metadata. + */ + private formatPathWithSource( + p: string, + metadata: Map, + ): string { + const meta = this.findMetadata(p, metadata); + if (meta) { + const shortPath = this.getShortPath(p, meta.source); + const scopeLabel = meta.scope === "global" ? "global" : meta.scope === "project" ? "project" : "temp"; + return `${meta.source} (${scopeLabel}) ${shortPath}`; + } + return this.formatDisplayPath(p); + } + + /** + * Format resource diagnostics with nice collision display using metadata. + */ + private formatDiagnostics( + diagnostics: readonly ResourceDiagnostic[], + metadata: Map, + ): string { + const lines: string[] = []; + + // Group collision diagnostics by name + const collisions = new Map(); + const otherDiagnostics: ResourceDiagnostic[] = []; + + for (const d of diagnostics) { + if (d.type === "collision" && d.collision) { + const list = collisions.get(d.collision.name) ?? []; + list.push(d); + collisions.set(d.collision.name, list); + } else { + otherDiagnostics.push(d); + } + } + + // Format collision diagnostics grouped by name + for (const [name, collisionList] of collisions) { + const first = collisionList[0]?.collision; + if (!first) continue; + lines.push(theme.fg("warning", ` "${name}" collision:`)); + // Show winner + lines.push( + theme.fg("dim", ` ${theme.fg("success", "✓")} ${this.formatPathWithSource(first.winnerPath, metadata)}`), + ); + // Show all losers + for (const d of collisionList) { + if (d.collision) { + lines.push( + theme.fg( + "dim", + ` ${theme.fg("warning", "✗")} ${this.formatPathWithSource(d.collision.loserPath, metadata)} (skipped)`, + ), + ); + } + } + } + + // Format other diagnostics (skill name collisions, parse errors, etc.) + for (const d of otherDiagnostics) { + if (d.path) { + // Use metadata-aware formatting for paths + const sourceInfo = this.formatPathWithSource(d.path, metadata); + lines.push(theme.fg(d.type === "error" ? "error" : "warning", ` ${sourceInfo}`)); + lines.push(theme.fg(d.type === "error" ? "error" : "warning", ` ${d.message}`)); + } else { + lines.push(theme.fg(d.type === "error" ? "error" : "warning", ` ${d.message}`)); + } + } + + return lines.join("\n"); + } + private showLoadedResources(options?: { extensionPaths?: string[]; force?: boolean }): void { const shouldShow = options?.force || this.options.verbose || !this.settingsManager.getQuietStartup(); if (!shouldShow) { return; } + const metadata = this.session.resourceLoader.getPathMetadata(); + + const sectionHeader = (name: string) => theme.fg("muted", `[${name}]`); + const contextFiles = this.session.resourceLoader.getAgentsFiles().agentsFiles; if (contextFiles.length > 0) { const contextList = contextFiles.map((f) => theme.fg("dim", ` ${this.formatDisplayPath(f.path)}`)).join("\n"); - this.chatContainer.addChild(new Text(theme.fg("muted", "Loaded context:\n") + contextList, 0, 0)); + this.chatContainer.addChild(new Text(`${sectionHeader("Context")}\n${contextList}`, 0, 0)); this.chatContainer.addChild(new Spacer(1)); } const skills = this.session.skills; if (skills.length > 0) { - const skillList = skills.map((s) => theme.fg("dim", ` ${this.formatDisplayPath(s.filePath)}`)).join("\n"); - this.chatContainer.addChild(new Text(theme.fg("muted", "Loaded skills:\n") + skillList, 0, 0)); + const skillPaths = skills.map((s) => s.filePath); + const groups = this.groupPathsBySource(skillPaths, metadata); + const skillList = this.formatGroupedPaths(groups, (p, source) => this.getShortPath(p, source)); + this.chatContainer.addChild(new Text(`${sectionHeader("Skills")}\n${skillList}`, 0, 0)); this.chatContainer.addChild(new Spacer(1)); } - const skillWarnings = this.session.skillWarnings; - if (skillWarnings.length > 0) { - const warningList = skillWarnings - .map((w) => theme.fg("warning", ` ${this.formatDisplayPath(w.skillPath)}: ${w.message}`)) - .join("\n"); - this.chatContainer.addChild(new Text(theme.fg("warning", "Skill warnings:\n") + warningList, 0, 0)); + const skillDiagnostics = this.session.skillWarnings; + if (skillDiagnostics.length > 0) { + const warningLines = this.formatDiagnostics(skillDiagnostics, metadata); + this.chatContainer.addChild(new Text(`${theme.fg("warning", "[Skill conflicts]")}\n${warningLines}`, 0, 0)); this.chatContainer.addChild(new Spacer(1)); } const templates = this.session.promptTemplates; if (templates.length > 0) { - const templateList = templates.map((t) => theme.fg("dim", ` /${t.name} ${t.source}`)).join("\n"); - this.chatContainer.addChild(new Text(theme.fg("muted", "Loaded prompt templates:\n") + templateList, 0, 0)); + // Group templates by source using metadata + const templatePaths = templates.map((t) => t.filePath); + const groups = this.groupPathsBySource(templatePaths, metadata); + const templateLines: string[] = []; + for (const [source, { scope, paths }] of groups) { + const scopeLabel = scope === "global" ? "global" : scope === "project" ? "project" : ""; + const sourceColor = source === "local" ? "muted" : "accent"; + const header = scopeLabel + ? `${theme.fg(sourceColor, source)} ${theme.fg("dim", `(${scopeLabel})`)}` + : theme.fg(sourceColor, source); + templateLines.push(` ${header}`); + for (const p of paths) { + const template = templates.find((t) => t.filePath === p); + if (template) { + templateLines.push(theme.fg("dim", ` /${template.name}`)); + } + } + } + this.chatContainer.addChild(new Text(`${sectionHeader("Prompts")}\n${templateLines.join("\n")}`, 0, 0)); + this.chatContainer.addChild(new Spacer(1)); + } + + const promptDiagnostics = this.session.resourceLoader.getPrompts().diagnostics; + if (promptDiagnostics.length > 0) { + const warningLines = this.formatDiagnostics(promptDiagnostics, metadata); + this.chatContainer.addChild(new Text(`${theme.fg("warning", "[Prompt conflicts]")}\n${warningLines}`, 0, 0)); this.chatContainer.addChild(new Spacer(1)); } const extensionPaths = options?.extensionPaths ?? []; if (extensionPaths.length > 0) { - const extList = extensionPaths.map((p) => theme.fg("dim", ` ${this.formatDisplayPath(p)}`)).join("\n"); - this.chatContainer.addChild(new Text(theme.fg("muted", "Loaded extensions:\n") + extList, 0, 0)); + const groups = this.groupPathsBySource(extensionPaths, metadata); + const extList = this.formatGroupedPaths(groups, (p, source) => this.getShortPath(p, source)); + this.chatContainer.addChild(new Text(`${sectionHeader("Extensions")}\n${extList}`, 0, 0)); + this.chatContainer.addChild(new Spacer(1)); + } + + // Show loaded themes (excluding built-in) + const loadedThemes = this.session.resourceLoader.getThemes().themes; + const customThemes = loadedThemes.filter((t) => t.sourcePath); + if (customThemes.length > 0) { + const themePaths = customThemes.map((t) => t.sourcePath!); + const groups = this.groupPathsBySource(themePaths, metadata); + const themeList = this.formatGroupedPaths(groups, (p, source) => this.getShortPath(p, source)); + this.chatContainer.addChild(new Text(`${sectionHeader("Themes")}\n${themeList}`, 0, 0)); + this.chatContainer.addChild(new Spacer(1)); + } + + const themeDiagnostics = this.session.resourceLoader.getThemes().diagnostics; + if (themeDiagnostics.length > 0) { + const warningLines = this.formatDiagnostics(themeDiagnostics, metadata); + this.chatContainer.addChild(new Text(`${theme.fg("warning", "[Theme conflicts]")}\n${warningLines}`, 0, 0)); this.chatContainer.addChild(new Spacer(1)); } } diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index f580f513..e849a9a2 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -411,4 +411,47 @@ Content`, expect(result.extensions.some((p) => p.endsWith("two.ts"))).toBe(false); }); }); + + describe("package deduplication", () => { + it("should dedupe same local package in global and project (project wins)", async () => { + const pkgDir = join(tempDir, "shared-pkg"); + mkdirSync(join(pkgDir, "extensions"), { recursive: true }); + writeFileSync(join(pkgDir, "extensions", "shared.ts"), "export default function() {}"); + + // Same package in both global and project + settingsManager.setPackages([pkgDir]); // global + settingsManager.setProjectPackages([pkgDir]); // project + + // Debug: verify settings are stored correctly + const globalSettings = settingsManager.getGlobalSettings(); + const projectSettings = settingsManager.getProjectSettings(); + expect(globalSettings.packages).toEqual([pkgDir]); + expect(projectSettings.packages).toEqual([pkgDir]); + + const result = await packageManager.resolve(); + // Auto-discovery returns directories, not individual files + // Should only appear once (deduped), with project scope + const sharedPaths = result.extensions.filter((p) => p.includes("shared-pkg")); + expect(sharedPaths.length).toBe(1); + const meta = result.metadata.get(sharedPaths[0]); + expect(meta?.scope).toBe("project"); + }); + + it("should keep both if different packages", async () => { + const pkg1Dir = join(tempDir, "pkg1"); + const pkg2Dir = join(tempDir, "pkg2"); + mkdirSync(join(pkg1Dir, "extensions"), { recursive: true }); + mkdirSync(join(pkg2Dir, "extensions"), { recursive: true }); + writeFileSync(join(pkg1Dir, "extensions", "from-pkg1.ts"), "export default function() {}"); + writeFileSync(join(pkg2Dir, "extensions", "from-pkg2.ts"), "export default function() {}"); + + settingsManager.setPackages([pkg1Dir]); // global + settingsManager.setProjectPackages([pkg2Dir]); // project + + const result = await packageManager.resolve(); + // Auto-discovery returns directories, not individual files + expect(result.extensions.some((p) => p.includes("pkg1"))).toBe(true); + expect(result.extensions.some((p) => p.includes("pkg2"))).toBe(true); + }); + }); }); diff --git a/packages/coding-agent/test/sdk-skills.test.ts b/packages/coding-agent/test/sdk-skills.test.ts index 4934ebbb..9f604b10 100644 --- a/packages/coding-agent/test/sdk-skills.test.ts +++ b/packages/coding-agent/test/sdk-skills.test.ts @@ -58,6 +58,7 @@ This is a test skill. getAgentsFiles: () => ({ agentsFiles: [] }), getSystemPrompt: () => undefined, getAppendSystemPrompt: () => [], + getPathMetadata: () => new Map(), reload: async () => {}, }; @@ -89,6 +90,7 @@ This is a test skill. getAgentsFiles: () => ({ agentsFiles: [] }), getSystemPrompt: () => undefined, getAppendSystemPrompt: () => [], + getPathMetadata: () => new Map(), reload: async () => {}, }; diff --git a/packages/coding-agent/test/utilities.ts b/packages/coding-agent/test/utilities.ts index 2b7c8c2d..6e4e60b4 100644 --- a/packages/coding-agent/test/utilities.ts +++ b/packages/coding-agent/test/utilities.ts @@ -183,6 +183,7 @@ export function createTestResourceLoader(): ResourceLoader { getAgentsFiles: () => ({ agentsFiles: [] }), getSystemPrompt: () => undefined, getAppendSystemPrompt: () => [], + getPathMetadata: () => new Map(), reload: async () => {}, }; } diff --git a/packages/mom/src/agent.ts b/packages/mom/src/agent.ts index cafe4bce..2733dbd5 100644 --- a/packages/mom/src/agent.ts +++ b/packages/mom/src/agent.ts @@ -458,6 +458,7 @@ function createRunner(sandboxConfig: SandboxConfig, channelId: string, channelDi getAgentsFiles: () => ({ agentsFiles: [] }), getSystemPrompt: () => systemPrompt, getAppendSystemPrompt: () => [], + getPathMetadata: () => new Map(), reload: async () => {}, };