From fcdbe24d6c800888fbae14e4d627c3f3fce6da1c Mon Sep 17 00:00:00 2001 From: harshitsinghbhandari <24b4506@iitb.ac.in> Date: Thu, 2 Apr 2026 21:28:53 +0530 Subject: [PATCH] fix: prevent one project's bad config from breaking shared plugin When multiple projects share the same external plugin, a misconfiguration in one project (wrong expectedPluginName) would prevent the plugin from being registered at all, silently breaking it for all projects. Fix by: 1. Register the plugin FIRST, before validating individual project configs 2. Validate each project's config in its own try-catch 3. Log validation errors but continue processing other projects This ensures: - Plugin is always registered if it loads successfully - Projects with correct config get updated - Projects with bad config get a warning but don't break others - Misconfigured projects fail at point of use with clearer errors Co-Authored-By: Claude Opus 4.5 --- .../src/__tests__/plugin-registry.test.ts | 82 ++++++++++++++++++- packages/core/src/plugin-registry.ts | 25 ++++-- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/packages/core/src/__tests__/plugin-registry.test.ts b/packages/core/src/__tests__/plugin-registry.test.ts index d5b75dbe6..51c0d2994 100644 --- a/packages/core/src/__tests__/plugin-registry.test.ts +++ b/packages/core/src/__tests__/plugin-registry.test.ts @@ -530,7 +530,7 @@ describe("External plugin manifest validation", () => { expect(registry.get("tracker", "jira")).not.toBeNull(); }); - it("warns when manifest.name does not match expectedPluginName", async () => { + it("warns when manifest.name does not match expectedPluginName but still registers plugin", async () => { const registry = createPluginRegistry(); const mockPlugin = { @@ -566,12 +566,16 @@ describe("External plugin manifest validation", () => { ], }); - // Should warn but not throw (error is caught and logged) + // Should warn about validation failure but still register the plugin const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); await registry.loadFromConfig(config, importFn); + // Plugin should still be registered under its manifest.name + expect(registry.get("tracker", "jira-enterprise")).not.toBeNull(); + + // Should have logged a validation warning expect(stderrSpy).toHaveBeenCalledWith( - expect.stringContaining("Failed to load plugin"), + expect.stringContaining("Config validation failed for projects.proj1.tracker"), ); stderrSpy.mockRestore(); }); @@ -771,4 +775,76 @@ describe("External plugin manifest validation", () => { // Plugin should be registered under manifest.name expect(registry.get("tracker", "jira-cloud")).not.toBeNull(); }); + + it("registers plugin even when one project has misconfigured expectedPluginName", 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 but with WRONG explicit plugin name + tracker: { plugin: "wrong-name", 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", + expectedPluginName: "wrong-name", // Mismatches manifest.name "jira-cloud" + }, + ], + }); + + const stderrSpy = vi.spyOn(process.stderr, "write").mockImplementation(() => true); + await registry.loadFromConfig(config, importFn); + + // Plugin should STILL be registered despite proj2's misconfiguration + expect(registry.get("tracker", "jira-cloud")).not.toBeNull(); + + // proj1 should be updated correctly (no expectedPluginName = accepts any) + expect(config.projects.proj1.tracker?.plugin).toBe("jira-cloud"); + + // proj2's config should NOT be updated (validation failed) + expect(config.projects.proj2.tracker?.plugin).toBe("wrong-name"); + + // Should have logged a warning about proj2's validation failure + expect(stderrSpy).toHaveBeenCalledWith( + expect.stringContaining("Config validation failed for projects.proj2.tracker"), + ); + + stderrSpy.mockRestore(); + }); }); diff --git a/packages/core/src/plugin-registry.ts b/packages/core/src/plugin-registry.ts index 14fc7b563..75231f41e 100644 --- a/packages/core/src/plugin-registry.ts +++ b/packages/core/src/plugin-registry.ts @@ -365,18 +365,29 @@ export function createPluginRegistry(): PluginRegistry { const mod = normalizeImportedPluginModule(await doImport(specifier)); if (!mod) continue; + // Register the plugin FIRST, before validating individual project configs. + // This ensures the plugin is available even if some projects have misconfigurations. + const pluginConfig = extractPluginConfig(mod.manifest.slot, mod.manifest.name, config); + this.register(mod, pluginConfig); + // Check if this plugin was auto-added from inline tracker/scm/notifier config // 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 - updateConfigWithManifestName(mod.manifest, externalEntry, config); + try { + // Validate manifest.name matches expectedPluginName (if specified) + validateManifestName(mod.manifest, externalEntry, specifier); + // Update the config with the actual manifest.name + updateConfigWithManifestName(mod.manifest, externalEntry, config); + } catch (validationError) { + // Log validation errors but don't abort - other projects can still use the plugin. + // The misconfigured project will fail later when it tries to use the plugin + // with the wrong name, giving a clearer error at point of use. + process.stderr.write( + `[plugin-registry] Config validation failed for ${externalEntry.source}: ${validationError}\n`, + ); + } } - - const pluginConfig = extractPluginConfig(mod.manifest.slot, mod.manifest.name, config); - this.register(mod, pluginConfig); } catch (error) { process.stderr.write(`[plugin-registry] Failed to load plugin "${specifier}": ${error}\n`); }