diff --git a/packages/core/src/__tests__/config-validation.test.ts b/packages/core/src/__tests__/config-validation.test.ts index 2703154e7..be7f9a0a2 100644 --- a/packages/core/src/__tests__/config-validation.test.ts +++ b/packages/core/src/__tests__/config-validation.test.ts @@ -1032,4 +1032,22 @@ describe("External Plugin Name Generation", () => { expect(config.projects.proj1.tracker?.plugin).toBe("my-custom-name"); }); + + it("handles local path without slashes correctly", () => { + const config = validateConfig({ + projects: { + proj1: { + path: "/repos/test", + repo: "org/test", + defaultBranch: "main", + tracker: { + path: "my-tracker", + }, + }, + }, + }); + + // Should use the path as-is (not split by hyphens like npm packages) + expect(config.projects.proj1.tracker?.plugin).toBe("my-tracker"); + }); }); diff --git a/packages/core/src/__tests__/plugin-registry.test.ts b/packages/core/src/__tests__/plugin-registry.test.ts index b34907b73..424492ab5 100644 --- a/packages/core/src/__tests__/plugin-registry.test.ts +++ b/packages/core/src/__tests__/plugin-registry.test.ts @@ -535,14 +535,13 @@ describe("External plugin manifest validation", () => { }); // Should warn but not throw (error is caught and logged) - const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); await registry.loadFromConfig(config, importFn); - expect(consoleSpy).toHaveBeenCalledWith( + expect(stderrSpy).toHaveBeenCalledWith( expect.stringContaining("Failed to load plugin"), - expect.any(Error), ); - consoleSpy.mockRestore(); + stderrSpy.mockRestore(); }); it("infers plugin name when expectedPluginName is not specified", async () => { @@ -671,12 +670,73 @@ describe("External plugin manifest validation", () => { ], }); - const consoleSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); await registry.loadFromConfig(config, importFn); - expect(consoleSpy).toHaveBeenCalledWith( + expect(stderrSpy).toHaveBeenCalledWith( expect.stringContaining("has slot \"notifier\" but was configured as \"tracker\""), ); - consoleSpy.mockRestore(); + stderrSpy.mockRestore(); + }); + + it("updates all projects sharing same external plugin with manifest.name", async () => { + const registry = createPluginRegistry(); + + const mockPlugin = { + manifest: { name: "jira-cloud", slot: "tracker" as const, version: "1.0.0", description: "Jira Cloud" }, + create: vi.fn(() => ({})), + }; + + const importFn = vi.fn(async () => mockPlugin); + + const config = makeOrchestratorConfig({ + configPath: "/test/config.yaml", + plugins: [ + { name: "jira", source: "npm", package: "@acme/ao-plugin-tracker-jira", enabled: true }, + ], + projects: { + proj1: { + path: "/repos/test1", + repo: "org/test1", + name: "proj1", + defaultBranch: "main", + sessionPrefix: "test1", + tracker: { plugin: "jira", package: "@acme/ao-plugin-tracker-jira" }, + }, + proj2: { + path: "/repos/test2", + repo: "org/test2", + name: "proj2", + defaultBranch: "main", + sessionPrefix: "test2", + // Same external plugin as proj1 + tracker: { plugin: "jira", package: "@acme/ao-plugin-tracker-jira" }, + }, + }, + _externalPluginEntries: [ + { + source: "projects.proj1.tracker", + location: { kind: "project", projectId: "proj1", configType: "tracker" }, + slot: "tracker", + package: "@acme/ao-plugin-tracker-jira", + // No expectedPluginName - will accept any manifest.name + }, + { + source: "projects.proj2.tracker", + location: { kind: "project", projectId: "proj2", configType: "tracker" }, + slot: "tracker", + package: "@acme/ao-plugin-tracker-jira", + // No expectedPluginName - will accept any manifest.name + }, + ], + }); + + await registry.loadFromConfig(config, importFn); + + // Both projects should be updated with the actual manifest.name + expect(config.projects.proj1.tracker?.plugin).toBe("jira-cloud"); + expect(config.projects.proj2.tracker?.plugin).toBe("jira-cloud"); + // Plugin should be registered under manifest.name + expect(registry.get("tracker", "jira-cloud")).not.toBeNull(); }); }); diff --git a/packages/core/src/config.ts b/packages/core/src/config.ts index 387e493be..7c09a23f9 100644 --- a/packages/core/src/config.ts +++ b/packages/core/src/config.ts @@ -313,22 +313,26 @@ function expandPaths(config: OrchestratorConfig): OrchestratorConfig { * Format: extract the last segment from the package/path, removing common prefixes. * e.g., "@acme/ao-plugin-tracker-jira" -> "jira" * e.g., "./plugins/my-tracker" -> "my-tracker" + * e.g., "my-tracker" (local path without slashes) -> "my-tracker" */ function generateTempPluginName(pkg?: string, path?: string): string { - const specifier = pkg ?? path ?? "unknown"; - - // For npm packages, extract the last part after the last hyphen or slash + // Handle npm packages: extract the last part after the last hyphen // @acme/ao-plugin-tracker-jira -> jira // @composio/ao-plugin-scm-gitlab -> gitlab - if (specifier.startsWith("@") || !specifier.includes("/")) { - const parts = specifier.split(/[-/]/); - return parts[parts.length - 1] ?? specifier; + if (pkg) { + const parts = pkg.split(/[-/]/); + return parts[parts.length - 1] ?? pkg; } - // For local paths, use the basename + // Handle local paths: use the basename // ./plugins/my-tracker -> my-tracker - const segments = specifier.split("/").filter((s) => s && s !== "." && s !== ".."); - return segments[segments.length - 1] ?? specifier; + // my-tracker -> my-tracker (no slashes is still a valid path) + if (path) { + const segments = path.split("/").filter((s) => s && s !== "." && s !== ".."); + return segments[segments.length - 1] ?? path; + } + + return "unknown"; } /** diff --git a/packages/core/src/plugin-registry.ts b/packages/core/src/plugin-registry.ts index 9812e355d..6d5793f70 100644 --- a/packages/core/src/plugin-registry.ts +++ b/packages/core/src/plugin-registry.ts @@ -86,16 +86,20 @@ function extractPluginConfig( } /** - * Find the external plugin entry that matches a given plugin config. + * Find ALL external plugin entries that match a given plugin config. * Used for manifest.name validation when loading inline tracker/scm/notifier plugins. + * + * Returns all matching entries because multiple projects may share the same + * external plugin (same package/path), and all their configs need to be updated + * with the actual manifest.name. */ -function findExternalPluginEntry( +function findAllExternalPluginEntries( plugin: InstalledPluginConfig, externalEntries: ExternalPluginEntryRef[] | undefined, -): ExternalPluginEntryRef | undefined { - if (!externalEntries) return undefined; +): ExternalPluginEntryRef[] { + if (!externalEntries) return []; - return externalEntries.find((entry) => { + return externalEntries.filter((entry) => { if (plugin.package && entry.package === plugin.package) return true; if (plugin.path && entry.path === plugin.path) return true; return false; @@ -153,9 +157,9 @@ function updateConfigWithManifestName( // Also validate slot matches if (manifest.slot !== slot) { - console.warn( + process.stderr.write( `[plugin-registry] Plugin at ${source} has slot "${manifest.slot}" but was configured as "${slot}". ` + - `The plugin will be registered under its declared slot "${manifest.slot}".`, + `The plugin will be registered under its declared slot "${manifest.slot}".\n`, ); } } @@ -344,7 +348,7 @@ export function createPluginRegistry(): PluginRegistry { const specifier = resolvePluginSpecifier(plugin, config); if (!specifier) { - console.warn(`[plugin-registry] Could not resolve specifier for plugin "${plugin.name}" (source: ${plugin.source})`); + process.stderr.write(`[plugin-registry] Could not resolve specifier for plugin "${plugin.name}" (source: ${plugin.source})\n`); continue; } @@ -353,8 +357,9 @@ export function createPluginRegistry(): PluginRegistry { if (!mod) continue; // Check if this plugin was auto-added from inline tracker/scm/notifier config - const externalEntry = findExternalPluginEntry(plugin, externalEntries); - if (externalEntry) { + // Multiple projects may share the same external plugin, so find ALL matching entries + const matchingEntries = findAllExternalPluginEntries(plugin, externalEntries); + for (const externalEntry of matchingEntries) { // Validate manifest.name matches expectedPluginName (if specified) validateManifestName(mod.manifest, externalEntry, specifier); // Update the config with the actual manifest.name @@ -364,7 +369,7 @@ export function createPluginRegistry(): PluginRegistry { const pluginConfig = extractPluginConfig(mod.manifest.slot, mod.manifest.name, config); this.register(mod, pluginConfig); } catch (error) { - console.warn(`[plugin-registry] Failed to load plugin "${specifier}":`, error); + process.stderr.write(`[plugin-registry] Failed to load plugin "${specifier}": ${error}\n`); } } },