From 375d0bc4d62945c55e734bc5aa66e3a0b36dda7f Mon Sep 17 00:00:00 2001 From: Mario Zechner Date: Fri, 23 Jan 2026 20:58:17 +0100 Subject: [PATCH] fix(coding-agent): user filters now layer on top of manifest filters Previously, user filters completely replaced manifest filtering. Now: 1. Manifest patterns are applied first (defines what package provides) 2. User patterns are applied on top (narrows down further) This means if a manifest excludes 10 extensions and user adds one more exclusion, all 11 are excluded (not just the user's one). --- .../coding-agent/src/core/package-manager.ts | 59 ++++++++++--------- .../coding-agent/test/package-manager.test.ts | 38 ++++++++++++ 2 files changed, 68 insertions(+), 29 deletions(-) diff --git a/packages/coding-agent/src/core/package-manager.ts b/packages/coding-agent/src/core/package-manager.ts index 80ca94b1..2534526e 100644 --- a/packages/coding-agent/src/core/package-manager.ts +++ b/packages/coding-agent/src/core/package-manager.ts @@ -879,69 +879,70 @@ export class DefaultPackageManager implements PackageManager { } /** - * Apply filter patterns to a package's resources. - * Supports glob patterns and exclusions (! prefix). + * 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, - patterns: string[], + userPatterns: string[], resourceType: ResourceType, target: Set, ): void { - if (patterns.length === 0) { + if (userPatterns.length === 0) { + // Empty array = load none return; } - if (!hasPatterns(patterns)) { - // No patterns - just resolve paths directly - for (const entry of patterns) { - const resolved = resolve(packageRoot, entry); - if (existsSync(resolved)) { - this.addPath(target, resolved); - } - } - return; - } + // First get what manifest provides (with manifest patterns already applied) + const manifestFiles = this.collectManifestFilteredFiles(packageRoot, resourceType); - // Has patterns - enumerate all files and filter - const allFiles = this.collectAllPackageFiles(packageRoot, resourceType); - const filtered = applyPatterns(allFiles, patterns, packageRoot); + // Then apply user patterns on top + const filtered = applyPatterns(manifestFiles, userPatterns, packageRoot); for (const f of filtered) { this.addPath(target, f); } } /** - * Collect all files of a given resource type from a package. + * Collect files that the manifest provides, with manifest patterns applied. + * This is what the package makes available before user filtering. */ - private collectAllPackageFiles(packageRoot: string, resourceType: ResourceType): string[] { + private collectManifestFilteredFiles(packageRoot: string, resourceType: ResourceType): string[] { const manifest = this.readPiManifest(packageRoot); - // If manifest specifies paths, use those if (manifest) { - const manifestPaths = manifest[resourceType]; - if (manifestPaths && manifestPaths.length > 0) { - const files: string[] = []; - for (const p of manifestPaths) { - const resolved = resolve(packageRoot, p); + 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()) { - files.push(resolved); + allFiles.push(resolved); } else if (stats.isDirectory()) { if (resourceType === "skills") { - files.push(...collectSkillEntries(resolved)); + allFiles.push(...collectSkillEntries(resolved)); } else { - files.push(...collectFiles(resolved, FILE_PATTERNS[resourceType])); + allFiles.push(...collectFiles(resolved, FILE_PATTERNS[resourceType])); } } } catch { // Ignore errors } } - return files; + + // Apply manifest patterns (if any) + const manifestPatterns = manifestEntries.filter(isPattern); + if (manifestPatterns.length > 0) { + return applyPatterns(allFiles, manifestPatterns, packageRoot); + } + return allFiles; } } diff --git a/packages/coding-agent/test/package-manager.test.ts b/packages/coding-agent/test/package-manager.test.ts index a5b6f560..f580f513 100644 --- a/packages/coding-agent/test/package-manager.test.ts +++ b/packages/coding-agent/test/package-manager.test.ts @@ -285,6 +285,44 @@ Content`, }); describe("pattern filtering in package filters", () => { + it("should apply user filters on top of manifest filters (not replace)", async () => { + // Manifest excludes baz.ts, user excludes bar.ts + // Result should exclude BOTH + const pkgDir = join(tempDir, "layered-pkg"); + mkdirSync(join(pkgDir, "extensions"), { recursive: true }); + writeFileSync(join(pkgDir, "extensions", "foo.ts"), "export default function() {}"); + writeFileSync(join(pkgDir, "extensions", "bar.ts"), "export default function() {}"); + writeFileSync(join(pkgDir, "extensions", "baz.ts"), "export default function() {}"); + writeFileSync( + join(pkgDir, "package.json"), + JSON.stringify({ + name: "layered-pkg", + pi: { + extensions: ["extensions", "!**/baz.ts"], + }, + }), + ); + + // User filter adds exclusion for bar.ts + settingsManager.setPackages([ + { + source: pkgDir, + extensions: ["!**/bar.ts"], + skills: [], + prompts: [], + themes: [], + }, + ]); + + const result = await packageManager.resolve(); + // foo.ts should be included (not excluded by anyone) + expect(result.extensions.some((p) => p.endsWith("foo.ts"))).toBe(true); + // bar.ts should be excluded (by user) + expect(result.extensions.some((p) => p.endsWith("bar.ts"))).toBe(false); + // baz.ts should be excluded (by manifest) + expect(result.extensions.some((p) => p.endsWith("baz.ts"))).toBe(false); + }); + it("should exclude extensions from package with ! pattern", async () => { const pkgDir = join(tempDir, "pattern-pkg"); mkdirSync(join(pkgDir, "extensions"), { recursive: true });