From b3b8285bbd95782e44210f100bb4bbc9f7f8db9d Mon Sep 17 00:00:00 2001 From: Hope Hadfield Date: Tue, 23 Jun 2026 10:44:57 -0400 Subject: [PATCH 1/5] feat(install-dynamic-plugins): add enabled field in DP config Signed-off-by: Hope Hadfield --- .../src/installer-npm.ts | 4 +- .../src/installer-oci.ts | 3 +- .../install-dynamic-plugins/src/installer.ts | 3 +- .../src/merger-pre-merge.test.ts | 68 ++++++++++++ .../src/merger.test.ts | 38 +++++++ .../install-dynamic-plugins/src/merger.ts | 5 +- .../install-dynamic-plugins/src/types.test.ts | 101 +++++++++++++++++- .../install-dynamic-plugins/src/types.ts | 59 ++++++++++ 8 files changed, 274 insertions(+), 7 deletions(-) diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts index bc75bb2ecfd..bfbfcf09fe5 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts @@ -20,7 +20,7 @@ import { verifyIntegrity } from './integrity'; import { log } from './log'; import { run } from './run'; import { extractNpmPackage } from './tar-extract'; -import { CONFIG_HASH_FILE, type Plugin } from './types'; +import { CONFIG_HASH_FILE, isPluginDisabled, type Plugin } from './types'; import { markAsFresh } from './util'; export type NpmInstallResult = { @@ -45,7 +45,7 @@ export async function installNpmPlugin( skipIntegrity: boolean, installed: Map, ): Promise { - if (plugin.disabled) { + if (isPluginDisabled(plugin, log)) { return { pluginPath: null, pluginConfig: {} }; } const hash = plugin.plugin_hash; diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts index 454baa98749..e23eac8bfc9 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts @@ -23,6 +23,7 @@ import { CONFIG_HASH_FILE, effectivePullPolicy, IMAGE_HASH_FILE, + isPluginDisabled, type Plugin, PullPolicy, } from './types'; @@ -62,7 +63,7 @@ export async function installOciPlugin( imageCache: OciImageCache, installed: Map, ): Promise { - if (plugin.disabled) { + if (isPluginDisabled(plugin, log)) { return { pluginPath: null, pluginConfig: {} }; } const hash = plugin.plugin_hash; diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer.ts index 04b3f410e3b..5650d68c8cb 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer.ts @@ -51,6 +51,7 @@ import { type DynamicPluginsConfig, effectivePullPolicy, GLOBAL_CONFIG_FILENAME, + isPluginDisabled, LOCK_FILENAME, OCI_PROTO, type Plugin, @@ -374,7 +375,7 @@ function categorize(allPlugins: PluginMap): Categorized { const npm: Plugin[] = []; const skipped: Plugin[] = []; for (const plugin of Object.values(allPlugins)) { - if (plugin.disabled) { + if (isPluginDisabled(plugin, log)) { log(`\n======= Skipping disabled plugin ${plugin.package}`); continue; } diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts index 3d8f976c8a4..48642cc364d 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts @@ -262,4 +262,72 @@ describe('filterDisabledOciPlugins', () => { ); expect(out).toHaveLength(2); }); + + it('removes invalid OCI entries that are marked enabled: false', () => { + const plugins: PluginSpec[] = [ + { package: 'oci://bad spec', enabled: false }, + { package: 'oci://also bad' }, + ]; + const out = filterDisabledOciPlugins(plugins, new Set()); + expect(out.map(p => p.package)).toEqual(['oci://also bad']); + }); +}); + +describe('preMergeOciDisabledState — enabled field', () => { + it('enabled: false in main disables the registry', () => { + const include: PluginSpec[] = [ + { package: 'oci://registry.example.com/plugin:1.0', enabled: true }, + ]; + const main: PluginSpec[] = [ + { + package: 'oci://registry.example.com/plugin:{{inherit}}', + enabled: false, + }, + ]; + const result = preMergeOciDisabledState( + [['include.yaml', include]], + main, + 'main.yaml', + ); + expect(result.has('oci://registry.example.com/plugin')).toBe(true); + }); + + it('enabled: true in main re-enables a disabled include', () => { + const include: PluginSpec[] = [ + { package: 'oci://registry.example.com/plugin:1.0', enabled: false }, + ]; + const main: PluginSpec[] = [ + { + package: 'oci://registry.example.com/plugin:{{inherit}}', + enabled: true, + }, + ]; + const result = preMergeOciDisabledState( + [['include.yaml', include]], + main, + 'main.yaml', + ); + expect(result.has('oci://registry.example.com/plugin')).toBe(false); + }); + + it('enabled takes precedence when both enabled and disabled are set', () => { + const warn = jest + .spyOn(process.stdout, 'write') + .mockImplementation(() => true); + try { + const main: PluginSpec[] = [ + { + package: 'oci://registry.example.com/plugin:1.0', + enabled: true, + disabled: true, + }, + ]; + const result = preMergeOciDisabledState([], main, 'main.yaml'); + expect(result.has('oci://registry.example.com/plugin')).toBe(false); + const out = warn.mock.calls.map(args => String(args[0])).join('\n'); + expect(out).toMatch(/both 'enabled' and 'disabled'/); + } finally { + warn.mockRestore(); + } + }); }); diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts index 09e0a8de7b6..da047ff2d88 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts @@ -83,6 +83,44 @@ describe('mergePlugin — NPM', () => { expect(all.pkg?.last_modified_level).toBe(1); }); + it('overrides using the enabled field', async () => { + const all: PluginMap = {}; + await mergePlugin( + { package: 'pkg@1.0.0', enabled: true }, + all, + 'inc.yaml', + 0, + ); + await mergePlugin( + { package: 'pkg@2.0.0', enabled: false }, + all, + 'cfg.yaml', + 1, + ); + expect(all.pkg?.package).toBe('pkg@2.0.0'); + expect(all.pkg?.enabled).toBe(false); + expect(all.pkg?.last_modified_level).toBe(1); + }); + + it('handles enabled overriding disabled from a lower level', async () => { + const all: PluginMap = {}; + await mergePlugin( + { package: 'pkg@1.0.0', disabled: true }, + all, + 'inc.yaml', + 0, + ); + await mergePlugin( + { package: 'pkg@2.0.0', enabled: true }, + all, + 'cfg.yaml', + 1, + ); + expect(all.pkg?.package).toBe('pkg@2.0.0'); + expect(all.pkg?.enabled).toBe(true); + expect(all.pkg?.last_modified_level).toBe(1); + }); + it('raises on duplicates within the same level', async () => { const all: PluginMap = {}; await mergePlugin({ package: 'pkg@1.0.0' }, all, 'cfg.yaml', 0); diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts index ee457c755a5..3cdd6b67944 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts @@ -26,6 +26,7 @@ import { } from './oci-key'; import { type DynamicPluginsConfig, + isPluginDisabled, OCI_PROTO, type Plugin, type PluginMap, @@ -422,7 +423,7 @@ function processOciEntry( ): void { const pkg = plugin.package; if (typeof pkg !== 'string' || !pkg.startsWith(OCI_PROTO)) return; - const disabled = plugin.disabled === true; + const disabled = isPluginDisabled(plugin, log); const parsed = tryParseOciRegistryAndPath(pkg); if (!parsed) { logInvalidOciFormat(pkg, sourceFile, disabled); @@ -551,7 +552,7 @@ export function filterDisabledOciPlugins( log(`\n======= Disabling OCI plugin ${pkg}`); continue; } - if (!parsed && plugin.disabled === true) { + if (!parsed && isPluginDisabled(plugin)) { log(`\n======= Disabling OCI plugin ${pkg}`); continue; } diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.test.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.test.ts index 81ce8362669..b0da589c58a 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.test.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.test.ts @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -import { parseMaxEntrySize } from './types'; +import { isPluginDisabled, parseMaxEntrySize } from './types'; describe('parseMaxEntrySize', () => { const DEFAULT = 40_000_000; @@ -46,3 +46,102 @@ describe('parseMaxEntrySize', () => { expect(parseMaxEntrySize('NaN')).toBe(DEFAULT); }); }); + +describe('isPluginDisabled', () => { + it('returns false when neither enabled nor disabled is set', () => { + expect(isPluginDisabled({ package: 'pkg@1.0' })).toBe(false); + }); + + it('returns false when enabled: true', () => { + expect(isPluginDisabled({ package: 'pkg@1.0', enabled: true })).toBe(false); + }); + + it('returns true when enabled: false', () => { + expect(isPluginDisabled({ package: 'pkg@1.0', enabled: false })).toBe(true); + }); + + it('returns true when disabled: true (backward compat)', () => { + expect(isPluginDisabled({ package: 'pkg@1.0', disabled: true })).toBe(true); + }); + + it('returns false when disabled: false (backward compat)', () => { + expect(isPluginDisabled({ package: 'pkg@1.0', disabled: false })).toBe( + false, + ); + }); + + it('enabled takes precedence over disabled when both set (enabled: true, disabled: true)', () => { + const warnings: string[] = []; + const result = isPluginDisabled( + { package: 'pkg@1.0', enabled: true, disabled: true }, + msg => warnings.push(msg), + ); + expect(result).toBe(false); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/both 'enabled' and 'disabled'/); + }); + + it('enabled takes precedence over disabled when both set (enabled: false, disabled: false)', () => { + const warnings: string[] = []; + const result = isPluginDisabled( + { package: 'pkg@1.0', enabled: false, disabled: false }, + msg => warnings.push(msg), + ); + expect(result).toBe(true); + expect(warnings).toHaveLength(1); + }); + + it('does not warn when no callback provided', () => { + expect( + isPluginDisabled({ package: 'pkg@1.0', enabled: true, disabled: true }), + ).toBe(false); + }); + + it('treats non-boolean enabled as unset (string "false" is not false)', () => { + const warnings: string[] = []; + const result = isPluginDisabled( + { package: 'pkg@1.0', enabled: 'false' as unknown as boolean }, + msg => warnings.push(msg), + ); + expect(result).toBe(false); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/non-boolean 'enabled: false'/); + }); + + it('treats null enabled as unset', () => { + const warnings: string[] = []; + const result = isPluginDisabled( + { package: 'pkg@1.0', enabled: null as unknown as boolean }, + msg => warnings.push(msg), + ); + expect(result).toBe(false); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/non-boolean 'enabled: null'/); + }); + + it('treats non-boolean disabled as unset', () => { + const warnings: string[] = []; + const result = isPluginDisabled( + { package: 'pkg@1.0', disabled: 'true' as unknown as boolean }, + msg => warnings.push(msg), + ); + expect(result).toBe(false); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/non-boolean 'disabled: true'/); + }); + + it('falls back to valid disabled when enabled is non-boolean', () => { + const warnings: string[] = []; + const result = isPluginDisabled( + { + package: 'pkg@1.0', + enabled: 'yes' as unknown as boolean, + disabled: true, + }, + msg => warnings.push(msg), + ); + expect(result).toBe(true); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatch(/non-boolean 'enabled/); + }); +}); diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts index 75704dff195..cbc89917374 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts @@ -30,7 +30,17 @@ export type PullPolicy = (typeof PullPolicy)[keyof typeof PullPolicy]; */ export type PluginSpec = { package: string; + /** + * Recommended: Use `enabled` instead. + * When both `enabled` and `disabled` are present, `enabled` takes precedence. + */ disabled?: boolean; + /** + * Whether the plugin is active. Preferred over `disabled` (positive logic). + * When both `enabled` and `disabled` are present, `enabled` takes precedence + * and a warning is logged. + */ + enabled?: boolean; pullPolicy?: PullPolicy; forceDownload?: boolean; integrity?: string; @@ -112,3 +122,52 @@ export function effectivePullPolicy(plugin: { ? PullPolicy.ALWAYS : PullPolicy.IF_NOT_PRESENT; } + +/** + * Resolve the effective disabled state from the `enabled` and `disabled` + * fields on a plugin spec. Precedence rules (per RHIDP-11983): + * + * 1. When only `enabled` is set → `disabled = !enabled`. + * 2. When only `disabled` is set → use it directly (backward compat). + * 3. When both are set → `enabled` wins and a warning is emitted + * via the optional `warn` callback. + * 4. When neither is set → default to `false` (not disabled). + * + * Non-boolean values (e.g. `enabled: 'false'` from unquoted YAML) are + * treated as unset and a warning is emitted, preventing JS truthiness + * from silently flipping activation state. + * + * The `warn` callback receives the warning message string. Pass `log` or + * leave it out for silent resolution (unit tests, hashing). + */ +export function isPluginDisabled( + plugin: { package: string; disabled?: boolean; enabled?: boolean }, + warn?: (msg: string) => void, +): boolean { + const hasEnabled = typeof plugin.enabled === 'boolean'; + const hasDisabled = typeof plugin.disabled === 'boolean'; + + if (plugin.enabled !== undefined && !hasEnabled) { + warn?.( + `WARNING: Plugin ${plugin.package} has non-boolean 'enabled: ${String(plugin.enabled)}'. ` + + `Expected true or false; ignoring the field.`, + ); + } + if (plugin.disabled !== undefined && !hasDisabled) { + warn?.( + `WARNING: Plugin ${plugin.package} has non-boolean 'disabled: ${String(plugin.disabled)}'. ` + + `Expected true or false; ignoring the field.`, + ); + } + + if (hasEnabled && hasDisabled) { + warn?.( + `WARNING: Plugin ${plugin.package} specifies both 'enabled' and 'disabled'. ` + + `The 'enabled' field takes precedence; please use only 'enabled'.`, + ); + return !plugin.enabled; + } + if (hasEnabled) return !plugin.enabled; + if (hasDisabled) return plugin.disabled === true; + return false; +} From e1dcff231ce0801870e04c173e16549b49d8fcd7 Mon Sep 17 00:00:00 2001 From: Hope Hadfield Date: Tue, 23 Jun 2026 10:48:34 -0400 Subject: [PATCH 2/5] chore: add changeset Signed-off-by: Hope Hadfield --- .../install-dynamic-plugins/.changeset/petite-ants-yell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 workspaces/install-dynamic-plugins/.changeset/petite-ants-yell.md diff --git a/workspaces/install-dynamic-plugins/.changeset/petite-ants-yell.md b/workspaces/install-dynamic-plugins/.changeset/petite-ants-yell.md new file mode 100644 index 00000000000..54279131656 --- /dev/null +++ b/workspaces/install-dynamic-plugins/.changeset/petite-ants-yell.md @@ -0,0 +1,5 @@ +--- +'@red-hat-developer-hub/cli-module-install-dynamic-plugins': minor +--- + +Add enabled field in dynamic plugin config with backwards compatibility for disabled field From 2ef602d5f1bc5158caa09695d24ed73ddd4f1760 Mon Sep 17 00:00:00 2001 From: Hope Hadfield Date: Tue, 23 Jun 2026 14:13:59 -0400 Subject: [PATCH 3/5] chore: remove multiple warnings Signed-off-by: Hope Hadfield --- .../src/installer-npm.ts | 2 +- .../src/installer-oci.ts | 2 +- .../src/merger-pre-merge.test.ts | 27 +++++++------------ .../install-dynamic-plugins/src/merger.ts | 2 +- 4 files changed, 12 insertions(+), 21 deletions(-) diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts index bfbfcf09fe5..514dc9f231f 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts @@ -45,7 +45,7 @@ export async function installNpmPlugin( skipIntegrity: boolean, installed: Map, ): Promise { - if (isPluginDisabled(plugin, log)) { + if (isPluginDisabled(plugin)) { return { pluginPath: null, pluginConfig: {} }; } const hash = plugin.plugin_hash; diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts index e23eac8bfc9..5ba49c2d4e7 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts @@ -63,7 +63,7 @@ export async function installOciPlugin( imageCache: OciImageCache, installed: Map, ): Promise { - if (isPluginDisabled(plugin, log)) { + if (isPluginDisabled(plugin)) { return { pluginPath: null, pluginConfig: {} }; } const hash = plugin.plugin_hash; diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts index 48642cc364d..bdfaa0dada1 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts @@ -311,23 +311,14 @@ describe('preMergeOciDisabledState — enabled field', () => { }); it('enabled takes precedence when both enabled and disabled are set', () => { - const warn = jest - .spyOn(process.stdout, 'write') - .mockImplementation(() => true); - try { - const main: PluginSpec[] = [ - { - package: 'oci://registry.example.com/plugin:1.0', - enabled: true, - disabled: true, - }, - ]; - const result = preMergeOciDisabledState([], main, 'main.yaml'); - expect(result.has('oci://registry.example.com/plugin')).toBe(false); - const out = warn.mock.calls.map(args => String(args[0])).join('\n'); - expect(out).toMatch(/both 'enabled' and 'disabled'/); - } finally { - warn.mockRestore(); - } + const main: PluginSpec[] = [ + { + package: 'oci://registry.example.com/plugin:1.0', + enabled: true, + disabled: true, + }, + ]; + const result = preMergeOciDisabledState([], main, 'main.yaml'); + expect(result.has('oci://registry.example.com/plugin')).toBe(false); }); }); diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts index 3cdd6b67944..c1572e3a2a0 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts @@ -423,7 +423,7 @@ function processOciEntry( ): void { const pkg = plugin.package; if (typeof pkg !== 'string' || !pkg.startsWith(OCI_PROTO)) return; - const disabled = isPluginDisabled(plugin, log); + const disabled = isPluginDisabled(plugin); const parsed = tryParseOciRegistryAndPath(pkg); if (!parsed) { logInvalidOciFormat(pkg, sourceFile, disabled); From 233493f534f39ff18e97703f63095fb454e9630c Mon Sep 17 00:00:00 2001 From: Hope Hadfield Date: Tue, 23 Jun 2026 15:20:23 -0400 Subject: [PATCH 4/5] chore: clear opposite field in override Signed-off-by: Hope Hadfield --- .../packages/install-dynamic-plugins/src/merger.test.ts | 1 + .../packages/install-dynamic-plugins/src/merger.ts | 7 +++++++ 2 files changed, 8 insertions(+) diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts index da047ff2d88..c35b00f3d1f 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts @@ -118,6 +118,7 @@ describe('mergePlugin — NPM', () => { ); expect(all.pkg?.package).toBe('pkg@2.0.0'); expect(all.pkg?.enabled).toBe(true); + expect(all.pkg?.disabled).toBeUndefined(); expect(all.pkg?.last_modified_level).toBe(1); }); diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts index c1572e3a2a0..56f4198292f 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts @@ -298,6 +298,13 @@ function copyPluginFields( if (skipSet.has(k) || isForbiddenKey(k)) continue; safeSet(dst, k, v); } + // When the override introduces one activation field, clear the opposite + // so the merged record never carries both (which would trigger a spurious + // "specifies both 'enabled' and 'disabled'" warning in isPluginDisabled). + if ('enabled' in src && 'disabled' in dst) + delete (dst as Record).disabled; + if ('disabled' in src && 'enabled' in dst) + delete (dst as Record).enabled; } function isEqual(a: unknown, b: unknown): boolean { From 2ae913d7b31ad9faa0d92f4944fe4c38db531681 Mon Sep 17 00:00:00 2001 From: Hope Hadfield Date: Tue, 23 Jun 2026 16:40:47 -0400 Subject: [PATCH 5/5] chore: address qodo review Signed-off-by: Hope Hadfield --- .../packages/install-dynamic-plugins/src/merger.ts | 12 +++++++----- .../packages/install-dynamic-plugins/src/types.ts | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts index 56f4198292f..dbcd2d2e549 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts @@ -298,12 +298,14 @@ function copyPluginFields( if (skipSet.has(k) || isForbiddenKey(k)) continue; safeSet(dst, k, v); } - // When the override introduces one activation field, clear the opposite - // so the merged record never carries both (which would trigger a spurious - // "specifies both 'enabled' and 'disabled'" warning in isPluginDisabled). - if ('enabled' in src && 'disabled' in dst) + // When the override introduces a valid boolean activation field, clear the + // opposite so the merged record never carries both (which would trigger a + // spurious "specifies both" warning in isPluginDisabled). Only act on + // actual booleans — a non-boolean value is treated as "unset" by + // isPluginDisabled and should not displace a valid value on dst. + if (typeof src.enabled === 'boolean' && 'disabled' in dst) delete (dst as Record).disabled; - if ('disabled' in src && 'enabled' in dst) + if (typeof src.disabled === 'boolean' && 'enabled' in dst) delete (dst as Record).enabled; } diff --git a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts index cbc89917374..0009c165947 100644 --- a/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts +++ b/workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts @@ -133,9 +133,9 @@ export function effectivePullPolicy(plugin: { * via the optional `warn` callback. * 4. When neither is set → default to `false` (not disabled). * - * Non-boolean values (e.g. `enabled: 'false'` from unquoted YAML) are - * treated as unset and a warning is emitted, preventing JS truthiness - * from silently flipping activation state. + * Non-boolean values (e.g. the quoted string `enabled: 'false'` or + * `enabled: null`) are treated as unset and a warning is emitted, + * preventing JS truthiness from silently flipping activation state. * * The `warn` callback receives the warning message string. Pass `log` or * leave it out for silent resolution (unit tests, hashing).