mirror of
https://github.com/getcompanion-ai/co-mono.git
synced 2026-04-15 13:03:42 +00:00
fix(coding-agent): multi-file extensions in packages now discovered correctly
Package resolution now uses the same discovery logic as local extensions: only index.ts (or manifest-declared entries) are loaded from subdirectories, not helper modules. fixes #1102
This commit is contained in:
parent
022b20f364
commit
ed80ab2129
3 changed files with 126 additions and 13 deletions
|
|
@ -2,6 +2,10 @@
|
|||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Multi-file extensions in packages now work correctly. Package resolution now uses the same discovery logic as local extensions: only `index.ts` (or manifest-declared entries) are loaded from subdirectories, not helper modules. ([#1102](https://github.com/badlogic/pi-mono/issues/1102))
|
||||
|
||||
## [0.50.6] - 2026-01-30
|
||||
|
||||
### Added
|
||||
|
|
|
|||
|
|
@ -454,6 +454,20 @@ function collectAutoExtensionEntries(dir: string): string[] {
|
|||
return entries;
|
||||
}
|
||||
|
||||
/**
|
||||
* Collect resource files from a directory based on resource type.
|
||||
* Extensions use smart discovery (index.ts in subdirs), others use recursive collection.
|
||||
*/
|
||||
function collectResourceFiles(dir: string, resourceType: ResourceType): string[] {
|
||||
if (resourceType === "skills") {
|
||||
return collectSkillEntries(dir);
|
||||
}
|
||||
if (resourceType === "extensions") {
|
||||
return collectAutoExtensionEntries(dir);
|
||||
}
|
||||
return collectFiles(dir, FILE_PATTERNS[resourceType]);
|
||||
}
|
||||
|
||||
function matchesAnyPattern(filePath: string, patterns: string[], baseDir: string): boolean {
|
||||
const rel = relative(baseDir, filePath);
|
||||
const name = basename(filePath);
|
||||
|
|
@ -1212,8 +1226,7 @@ export class DefaultPackageManager implements PackageManager {
|
|||
const dir = join(packageRoot, resourceType);
|
||||
if (existsSync(dir)) {
|
||||
// Collect all files from the directory (all enabled by default)
|
||||
const files =
|
||||
resourceType === "skills" ? collectSkillEntries(dir) : collectFiles(dir, FILE_PATTERNS[resourceType]);
|
||||
const files = collectResourceFiles(dir, resourceType);
|
||||
for (const f of files) {
|
||||
this.addResource(this.getTargetMap(accumulator, resourceType), f, metadata, true);
|
||||
}
|
||||
|
|
@ -1238,8 +1251,7 @@ export class DefaultPackageManager implements PackageManager {
|
|||
const dir = join(packageRoot, resourceType);
|
||||
if (existsSync(dir)) {
|
||||
// Collect all files from the directory (all enabled by default)
|
||||
const files =
|
||||
resourceType === "skills" ? collectSkillEntries(dir) : collectFiles(dir, FILE_PATTERNS[resourceType]);
|
||||
const files = collectResourceFiles(dir, resourceType);
|
||||
for (const f of files) {
|
||||
this.addResource(target, f, metadata, true);
|
||||
}
|
||||
|
|
@ -1295,10 +1307,7 @@ export class DefaultPackageManager implements PackageManager {
|
|||
if (!existsSync(conventionDir)) {
|
||||
return { allFiles: [], enabledByManifest: new Set() };
|
||||
}
|
||||
const allFiles =
|
||||
resourceType === "skills"
|
||||
? collectSkillEntries(conventionDir)
|
||||
: collectFiles(conventionDir, FILE_PATTERNS[resourceType]);
|
||||
const allFiles = collectResourceFiles(conventionDir, resourceType);
|
||||
return { allFiles, enabledByManifest: new Set(allFiles) };
|
||||
}
|
||||
|
||||
|
|
@ -1495,11 +1504,7 @@ export class DefaultPackageManager implements PackageManager {
|
|||
if (stats.isFile()) {
|
||||
files.push(p);
|
||||
} else if (stats.isDirectory()) {
|
||||
if (resourceType === "skills") {
|
||||
files.push(...collectSkillEntries(p));
|
||||
} else {
|
||||
files.push(...collectFiles(p, FILE_PATTERNS[resourceType]));
|
||||
}
|
||||
files.push(...collectResourceFiles(p, resourceType));
|
||||
}
|
||||
} catch {
|
||||
// Ignore errors
|
||||
|
|
|
|||
|
|
@ -710,4 +710,108 @@ Content`,
|
|||
expect(result.extensions.some((r) => r.path.includes("pkg2"))).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("multi-file extension discovery (issue #1102)", () => {
|
||||
it("should only load index.ts from subdirectories, not helper modules", async () => {
|
||||
// Regression test: packages with multi-file extensions in subdirectories
|
||||
// should only load the index.ts entry point, not helper modules like agents.ts
|
||||
const pkgDir = join(tempDir, "multifile-pkg");
|
||||
mkdirSync(join(pkgDir, "extensions", "subagent"), { recursive: true });
|
||||
|
||||
// Main entry point
|
||||
writeFileSync(
|
||||
join(pkgDir, "extensions", "subagent", "index.ts"),
|
||||
`import { helper } from "./agents.js";
|
||||
export default function(api) { api.registerTool({ name: "test", description: "test", execute: async () => helper() }); }`,
|
||||
);
|
||||
// Helper module (should NOT be loaded as standalone extension)
|
||||
writeFileSync(
|
||||
join(pkgDir, "extensions", "subagent", "agents.ts"),
|
||||
`export function helper() { return "helper"; }`,
|
||||
);
|
||||
// Top-level extension file (should be loaded)
|
||||
writeFileSync(join(pkgDir, "extensions", "standalone.ts"), "export default function(api) {}");
|
||||
|
||||
const result = await packageManager.resolveExtensionSources([pkgDir]);
|
||||
|
||||
// Should find the index.ts and standalone.ts
|
||||
expect(result.extensions.some((r) => r.path.endsWith("subagent/index.ts") && r.enabled)).toBe(true);
|
||||
expect(result.extensions.some((r) => r.path.endsWith("standalone.ts") && r.enabled)).toBe(true);
|
||||
|
||||
// Should NOT find agents.ts as a standalone extension
|
||||
expect(result.extensions.some((r) => r.path.endsWith("agents.ts"))).toBe(false);
|
||||
});
|
||||
|
||||
it("should respect package.json pi.extensions manifest in subdirectories", async () => {
|
||||
const pkgDir = join(tempDir, "manifest-subdir-pkg");
|
||||
mkdirSync(join(pkgDir, "extensions", "custom"), { recursive: true });
|
||||
|
||||
// Subdirectory with its own manifest
|
||||
writeFileSync(
|
||||
join(pkgDir, "extensions", "custom", "package.json"),
|
||||
JSON.stringify({
|
||||
pi: {
|
||||
extensions: ["./main.ts"],
|
||||
},
|
||||
}),
|
||||
);
|
||||
writeFileSync(join(pkgDir, "extensions", "custom", "main.ts"), "export default function(api) {}");
|
||||
writeFileSync(join(pkgDir, "extensions", "custom", "utils.ts"), "export const util = 1;");
|
||||
|
||||
const result = await packageManager.resolveExtensionSources([pkgDir]);
|
||||
|
||||
// Should find main.ts declared in manifest
|
||||
expect(result.extensions.some((r) => r.path.endsWith("custom/main.ts") && r.enabled)).toBe(true);
|
||||
|
||||
// Should NOT find utils.ts (not declared in manifest)
|
||||
expect(result.extensions.some((r) => r.path.endsWith("utils.ts"))).toBe(false);
|
||||
});
|
||||
|
||||
it("should handle mixed top-level files and subdirectories", async () => {
|
||||
const pkgDir = join(tempDir, "mixed-pkg");
|
||||
mkdirSync(join(pkgDir, "extensions", "complex"), { recursive: true });
|
||||
|
||||
// Top-level extension
|
||||
writeFileSync(join(pkgDir, "extensions", "simple.ts"), "export default function(api) {}");
|
||||
|
||||
// Subdirectory with index.ts + helpers
|
||||
writeFileSync(
|
||||
join(pkgDir, "extensions", "complex", "index.ts"),
|
||||
"import { a } from './a.js'; export default function(api) {}",
|
||||
);
|
||||
writeFileSync(join(pkgDir, "extensions", "complex", "a.ts"), "export const a = 1;");
|
||||
writeFileSync(join(pkgDir, "extensions", "complex", "b.ts"), "export const b = 2;");
|
||||
|
||||
const result = await packageManager.resolveExtensionSources([pkgDir]);
|
||||
|
||||
// Should find simple.ts and complex/index.ts
|
||||
expect(result.extensions.some((r) => r.path.endsWith("simple.ts") && r.enabled)).toBe(true);
|
||||
expect(result.extensions.some((r) => r.path.endsWith("complex/index.ts") && r.enabled)).toBe(true);
|
||||
|
||||
// Should NOT find helper modules
|
||||
expect(result.extensions.some((r) => r.path.endsWith("complex/a.ts"))).toBe(false);
|
||||
expect(result.extensions.some((r) => r.path.endsWith("complex/b.ts"))).toBe(false);
|
||||
|
||||
// Total should be exactly 2
|
||||
expect(result.extensions.filter((r) => r.enabled).length).toBe(2);
|
||||
});
|
||||
|
||||
it("should skip subdirectories without index.ts or manifest", async () => {
|
||||
const pkgDir = join(tempDir, "no-entry-pkg");
|
||||
mkdirSync(join(pkgDir, "extensions", "broken"), { recursive: true });
|
||||
|
||||
// Subdirectory with no index.ts and no manifest
|
||||
writeFileSync(join(pkgDir, "extensions", "broken", "helper.ts"), "export const x = 1;");
|
||||
writeFileSync(join(pkgDir, "extensions", "broken", "another.ts"), "export const y = 2;");
|
||||
|
||||
// Valid top-level extension
|
||||
writeFileSync(join(pkgDir, "extensions", "valid.ts"), "export default function(api) {}");
|
||||
|
||||
const result = await packageManager.resolveExtensionSources([pkgDir]);
|
||||
|
||||
// Should only find the valid top-level extension
|
||||
expect(result.extensions.some((r) => r.path.endsWith("valid.ts") && r.enabled)).toBe(true);
|
||||
expect(result.extensions.filter((r) => r.enabled).length).toBe(1);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue