Skip to content

feat(install-dynamic-plugins): add enabled field in DP config - #3554

Merged
hopehadfield merged 5 commits into
redhat-developer:mainfrom
hopehadfield:enabled-field
Jun 23, 2026
Merged

feat(install-dynamic-plugins): add enabled field in DP config#3554
hopehadfield merged 5 commits into
redhat-developer:mainfrom
hopehadfield:enabled-field

Conversation

@hopehadfield

Copy link
Copy Markdown
Member

Description

Duplicate of redhat-developer/rhdh#4937, which was dropped during migration.

Adds the enabled field with backwards compatibility

See redhat-developer/rhdh-plugin-export-overlays#2577 for relevant change in DPDY generation

Which issue(s) does this PR fix

Fixes RHIDP-11988D

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
@hopehadfield
hopehadfield requested review from a team as code owners June 23, 2026 14:49
@rhdh-gh-app

rhdh-gh-app Bot commented Jun 23, 2026

Copy link
Copy Markdown

Important

This PR includes changes that affect public-facing API. Please ensure you are adding/updating documentation for new features or behavior.

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/cli-module-install-dynamic-plugins workspaces/install-dynamic-plugins/packages/install-dynamic-plugins minor v0.2.0

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Summary by Qodo

Add enabled field with backward-compatible disable logic for dynamic plugins
✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

Description

• Add enabled to dynamic plugin specs while keeping disabled backward compatible.
• Centralize precedence and validation via isPluginDisabled() with warning logs.
• Update installers/merger and add tests covering merge and OCI filtering behavior.
Diagram

graph TD
  CFG["Dynamic plugins YAML"] --> MERGE["merger.ts"] --> UTIL["types.ts isPluginDisabled()"] --> INSTALL["installer.ts"] --> NPM["installer-npm.ts"]
  INSTALL --> OCI["installer-oci.ts"]
  TESTS["*.test.ts"] --> UTIL
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Normalize config up-front (convert to a single boolean field)
  • ➕ Downstream code only deals with one canonical field, simplifying call sites
  • ➕ Central place to enforce deprecation policy and emit warnings once
  • ➖ Requires a clear config parsing/normalization layer boundary
  • ➖ May be more invasive if specs are used in multiple forms (raw vs merged)
2. Schema validation (e.g., Zod/JSON schema) at config load time
  • ➕ Stronger guarantees (boolean types) before any merge/install logic runs
  • ➕ Better error reporting and easier future evolution of config format
  • ➖ Adds dependency/complexity and may require broader refactors
  • ➖ Needs alignment on whether invalid types should hard-fail vs warn-and-ignore

Recommendation: The chosen approach (a single isPluginDisabled() resolver used everywhere) is a good trade-off: it keeps backward compatibility, avoids scattering precedence logic, and reduces regression risk. If config evolution continues, consider adding a dedicated normalization/validation step so non-boolean YAML values can be caught earlier and optionally treated as configuration errors.

Files changed (9) +279 / -7

Enhancement (5) +68 / -6
installer-npm.tsSkip NPM installs using centralized disable resolution +2/-2

Skip NPM installs using centralized disable resolution

• Replaces direct 'plugin.disabled' checks with 'isPluginDisabled(plugin, log)'. Ensures NPM plugin installation respects the new 'enabled' field and warning behavior consistently.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-npm.ts

installer-oci.tsSkip OCI installs using centralized disable resolution +2/-1

Skip OCI installs using centralized disable resolution

• Replaces direct 'plugin.disabled' checks with 'isPluginDisabled(plugin, log)'. Aligns OCI installation behavior with the new 'enabled' field and legacy 'disabled' handling.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts

installer.tsUse 'isPluginDisabled()' when categorizing/skipping plugins +2/-1

Use 'isPluginDisabled()' when categorizing/skipping plugins

• Updates plugin categorization to skip inactive plugins via 'isPluginDisabled(plugin, log)' instead of checking only 'disabled'. This ensures consistent behavior across the overall install workflow.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer.ts

merger.tsApply 'isPluginDisabled()' in OCI processing and filtering paths +3/-2

Apply 'isPluginDisabled()' in OCI processing and filtering paths

• Replaces ad-hoc 'disabled === true' logic with 'isPluginDisabled(plugin, log)' when processing OCI entries and filtering disabled OCI plugins. This makes OCI handling honor 'enabled: false' and the defined precedence rules.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts

types.tsAdd 'enabled' to PluginSpec and introduce 'isPluginDisabled()' helper +59/-0

Add 'enabled' to PluginSpec and introduce 'isPluginDisabled()' helper

• Extends 'PluginSpec' with an 'enabled?: boolean' field and documents deprecation guidance for 'disabled'. Adds 'isPluginDisabled()' to centralize precedence rules, backward compatibility, and warning emission for conflicting or non-boolean fields.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts

Tests (3) +206 / -1
merger-pre-merge.test.tsAdd tests for 'enabled' handling during OCI pre-merge disable state +68/-0

Add tests for 'enabled' handling during OCI pre-merge disable state

• Adds coverage for removing invalid OCI entries when 'enabled: false' is set and for 'enabled' precedence during pre-merge registry disable/enable resolution. Includes a warning assertion when both 'enabled' and 'disabled' are present.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger-pre-merge.test.ts

merger.test.tsAdd merge precedence tests for 'enabled' overriding legacy 'disabled' +38/-0

Add merge precedence tests for 'enabled' overriding legacy 'disabled'

• Introduces new test cases validating that 'enabled' participates in merge overrides across config levels. Ensures 'enabled' can disable a previously enabled plugin and can re-enable a plugin disabled at a lower level.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.test.ts

types.test.tsAdd comprehensive unit tests for 'isPluginDisabled()' +100/-1

Add comprehensive unit tests for 'isPluginDisabled()'

• Adds tests for default behavior, 'enabled'/'disabled' cases, precedence when both are set, optional warning callback behavior, and non-boolean value handling. Ensures non-boolean YAML-derived values are ignored with warnings to avoid JS truthiness surprises.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.test.ts

Other (1) +5 / -0
petite-ants-yell.mdAdd changeset for new 'enabled' plugin config field +5/-0

Add changeset for new 'enabled' plugin config field

• Introduces a changeset bumping the install-dynamic-plugins CLI module as a minor change. Documents the addition of 'enabled' with backward compatibility for 'disabled'.

workspaces/install-dynamic-plugins/.changeset/petite-ants-yell.md

@codecov

codecov Bot commented Jun 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.64%. Comparing base (a32a667) to head (2ae913d).
⚠️ Report is 12 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3554   +/-   ##
=======================================
  Coverage   53.63%   53.64%           
=======================================
  Files        2260     2260           
  Lines       85976    85994   +18     
  Branches    24203    24209    +6     
=======================================
+ Hits        46117    46134   +17     
- Misses      38314    38315    +1     
  Partials     1545     1545           
Flag Coverage Δ *Carryforward flag
adoption-insights 83.70% <ø> (ø) Carriedforward from 233493f
ai-integrations 67.95% <ø> (ø) Carriedforward from 233493f
app-defaults 69.79% <ø> (ø) Carriedforward from 233493f
augment 46.39% <ø> (ø) Carriedforward from 233493f
boost 74.64% <ø> (ø) Carriedforward from 233493f
bulk-import 72.46% <ø> (ø) Carriedforward from 233493f
cost-management 14.10% <ø> (ø) Carriedforward from 233493f
dcm 61.79% <ø> (ø) Carriedforward from 233493f
extensions 61.53% <ø> (ø) Carriedforward from 233493f
global-floating-action-button 71.18% <ø> (ø) Carriedforward from 233493f
global-header 59.71% <ø> (ø) Carriedforward from 233493f
homepage 49.92% <ø> (ø) Carriedforward from 233493f
install-dynamic-plugins 56.77% <83.33%> (+0.53%) ⬆️
konflux 91.49% <ø> (ø) Carriedforward from 233493f
lightspeed 68.57% <ø> (ø) Carriedforward from 233493f
mcp-integrations 85.46% <ø> (ø) Carriedforward from 233493f
orchestrator 37.79% <ø> (ø) Carriedforward from 233493f
quickstart 63.76% <ø> (ø) Carriedforward from 233493f
sandbox 79.56% <ø> (ø) Carriedforward from 233493f
scorecard 83.96% <ø> (ø) Carriedforward from 233493f
theme 61.26% <ø> (ø) Carriedforward from 233493f
translations 7.25% <ø> (ø) Carriedforward from 233493f
x2a 78.68% <ø> (ø) Carriedforward from 233493f

*This pull request uses carry forward flags. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a32a667...2ae913d. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Jun 23, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (0) 📎 Requirement gaps (0) 📜 Skill insights (0)

Context used
✅ Tickets: RHIDP-11988

Grey Divider


Remediation recommended

1. Docs omit enabled field 🐞 Bug ⚙ Maintainability
Description
The PR adds enabled to the dynamic-plugins.yaml schema, but the package README still claims the
input schema is unchanged and only lists the legacy disabled flag. This can mislead users/tooling
about supported config and precedence rules.
Code

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts[R33-43]

+  /**
+   * 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;
Relevance

⭐⭐⭐ High

Docs drift is regularly fixed; README/config field alignment suggestions were accepted previously
(e.g., PRs #2861, #2647).

PR-#2861
PR-#2647

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PluginSpec now includes an enabled field, while the README still documents the schema as
containing disabled but not enabled, and states the input contract is unchanged.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts[31-48]
workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/README.md[122-126]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The README’s “Compatibility notes” still states the input schema matches the Python script exactly and lists only `disabled`, but the code now supports `enabled` (preferred) and precedence behavior.

### Issue Context
This PR introduces `enabled?: boolean` on `PluginSpec` and implements precedence via `isPluginDisabled(...)`.

### Fix Focus Areas
- workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/README.md[122-126]
- workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/types.ts[31-48]

### Expected change
Update the README schema list to include `enabled` (and ideally note that `enabled` is preferred and takes precedence over `disabled` when both are set).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

2. Stale pre-merge comment 🐞 Bug ⚙ Maintainability
Description
preMergeOciDisabledState now resolves disabled state via isPluginDisabled (which considers both
enabled and disabled), but its JSDoc still says it only inspects package and disabled. This
mismatch can cause incorrect assumptions during future refactors.
Code

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts[R423-427]

): 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);
Relevance

⭐⭐⭐ High

Team frequently accepts JSDoc/comment sync fixes (e.g., clarifying JSDoc accepted in PRs #2393,
#3442).

PR-#2393
PR-#3442

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The code path that computes disabled state for OCI entries now calls `isPluginDisabled(plugin,
log)`, but the JSDoc describing what the pre-merge pass inspects wasn’t updated accordingly.

workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts[418-430]
workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts[497-506]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The implementation now uses `isPluginDisabled(...)` (enabled+disabled precedence), but the JSDoc for `preMergeOciDisabledState` still claims it “Only inspects `package` and `disabled`”.

### Issue Context
This is documentation drift introduced by the new `enabled` field support and can confuse maintainers about what the pre-merge pass actually evaluates.

### Fix Focus Areas
- workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts[497-506]
- workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/merger.ts[418-430]

### Expected change
Update the JSDoc to state it inspects `package` and the effective disabled state derived from `enabled`/`disabled` (and optionally note the precedence and warning behavior).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

@rhdh-qodo-merge rhdh-qodo-merge Bot added enhancement New feature or request Tests labels Jun 23, 2026
Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
@gustavolira

Copy link
Copy Markdown
Member

/review
--pr_reviewer.inline_code_comments=true
-i
--pr_reviewer.require_score_review=true
--pr_reviewer.require_can_be_split_review=true
--pr_reviewer.num_code_suggestions="2"

@rhdh-qodo-merge

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Warning

/review is deprecated. Use /agentic_review instead (removal date not yet scheduled).

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🏅 Score: 88
🧪 PR contains tests
🔒 No security concerns identified
🔀 No multiple PR themes
⚡ Recommended focus areas for review

Possible Issue

In copyPluginFields, the logic that deletes the opposite activation field (enabled vs disabled) triggers purely on property presence ('enabled' in src / 'disabled' in src). If an override provides a non-boolean enabled/disabled value (which isPluginDisabled intentionally treats as “unset”), this deletion can still remove a valid boolean value from the destination, potentially changing the effective activation state unexpectedly.

function copyPluginFields(
  src: Plugin,
  dst: Plugin,
  skip: ReadonlyArray<string>,
): void {
  const skipSet = new Set<string>(skip);
  for (const [k, v] of Object.entries(src)) {
    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<string, unknown>).disabled;
  if ('disabled' in src && 'enabled' in dst)
    delete (dst as Record<string, unknown>).enabled;
Messaging

The warning comment about non-boolean values mentions “unquoted YAML” producing strings (e.g., enabled: 'false'); in YAML, unquoted false is boolean, while quoted 'false' is a string. Consider adjusting wording to avoid confusion and ensure the warnings guide users accurately.

/**
 * 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;
}
📚 Focus areas based on broader codebase context

Schema Drift

This PR introduces a new enabled field and adds precedence rules over the existing disabled field via isPluginDisabled. Existing config validation/test data in related code expects disabled as the canonical activation flag; adding enabled may require updating downstream config schema expectations and test fixtures to avoid silent mismatches (e.g., configs containing only enabled not being recognized by validators that only check disabled). (Ref 1)

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;
  pluginConfig?: Record<string, unknown>;
};

/**
 * Internal plugin record. Extends the YAML schema with fields populated at
 * runtime (`version` from the package string, `plugin_hash` for change
 * detection, `last_modified_level` to track include-file precedence).
 *
 * The field name `last_modified_level` matches the Python implementation so
 * the install hashes computed by `plugin-hash.ts` stay byte-compatible
 * across the Python ↔ TS migration. Renaming it would force every existing
 * dynamic-plugins-root to be re-installed on the first TS run.
 */
export type Plugin = PluginSpec & {
  version?: string;
  plugin_hash?: string;
  last_modified_level?: number;
};

export type PluginMap = Record<string, Plugin>;

export type DynamicPluginsConfig = {
  includes?: string[];
  plugins?: PluginSpec[];
};

export const DOCKER_PROTO = 'docker://';
export const OCI_PROTO = 'oci://';
/**
 * Tag suffix that, by convention, opts an OCI plugin into `pullPolicy: Always`
 * when no explicit policy is set — mirrors the Python script's behaviour and
 * keeps the two implementations swappable. Always parsed in combination with
 * the `!plugin-path` separator so a plugin tagged `:latest` (no plugin path)
 * does not accidentally trigger.
 */
export const LATEST_TAG_MARKER = ':latest!';
export const RHDH_REGISTRY = 'registry.access.redhat.com/rhdh/';
export const RHDH_FALLBACK = 'quay.io/rhdh/';
export const CONFIG_HASH_FILE = 'dynamic-plugin-config.hash';
export const IMAGE_HASH_FILE = 'dynamic-plugin-image.hash';
export const DPDY_FILENAME = 'dynamic-plugins.default.yaml';
export const LOCK_FILENAME = 'install-dynamic-plugins.lock';
export const GLOBAL_CONFIG_FILENAME = 'app-config.dynamic-plugins.yaml';

const DEFAULT_MAX_ENTRY_SIZE = 40_000_000;

/**
 * Parse the MAX_ENTRY_SIZE env var, falling back to the default when unset,
 * non-numeric, or < 1. Exported for unit tests — the `MAX_ENTRY_SIZE` constant
 * below is the module-level value used by the extractors.
 */
export function parseMaxEntrySize(
  raw: string | undefined = process.env.MAX_ENTRY_SIZE,
): number {
  if (!raw) return DEFAULT_MAX_ENTRY_SIZE;
  const n = Number.parseInt(raw, 10);
  return Number.isFinite(n) && n >= 1 ? n : DEFAULT_MAX_ENTRY_SIZE;
}

export const MAX_ENTRY_SIZE = parseMaxEntrySize();
export const RECOGNIZED_ALGORITHMS = ['sha512', 'sha384', 'sha256'] as const;
export type Algorithm = (typeof RECOGNIZED_ALGORITHMS)[number];

/**
 * Resolve the effective `pullPolicy` for an OCI plugin: an explicit policy
 * wins, otherwise the convention is `Always` for `:latest!` images and
 * `IfNotPresent` for everything else. Shared by the install pipeline and the
 * "definitely no-op" pre-pass so the `:latest!` semantics live in one place.
 */
export function effectivePullPolicy(plugin: {
  pullPolicy?: PullPolicy;
  package: string;
}): PullPolicy {
  if (plugin.pullPolicy) return plugin.pullPolicy;
  return plugin.package.includes(LATEST_TAG_MARKER)
    ? 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;
}

Reference reasoning: The referenced tests and example dynamic plugin configs use disabled as the supported/validated field and construct plugin specs accordingly, without any notion of enabled. Introducing enabled as preferred behavior means any other parts of the ecosystem that parse/validate dynamic plugin configs must be updated to account for enabled, or enabled-only configs will not be reflected in those validations.

📄 References
  1. redhat-developer/rhdh/dynamic-plugins/_utils/src/wrappers.test.ts [141-307]
  2. redhat-developer/rhdh/packages/backend/src/modules/rbacDynamicPluginsModule.ts [1-37]
  3. redhat-developer/rhdh/e2e-tests/playwright/support/test-data/dynamic-plugins-info.ts [1-52]
  4. redhat-developer/rhdh/plugins/dynamic-plugins-info-backend/src/plugin.ts [1-42]
  5. redhat-developer/rhdh/plugins/dynamic-plugins-info-backend/__fixtures__/data.ts [1-68]
  6. redhat-developer/rhdh/plugins/dynamic-plugins-info-backend/__fixtures__/data.ts [77-143]
  7. redhat-developer/rhdh/plugins/dynamic-plugins-info-backend/__fixtures__/data.ts [69-77]
  8. redhat-developer/rhdh/plugins/dynamic-plugins-info-backend/__fixtures__/expected_result.ts [62-87]

Signed-off-by: Hope Hadfield <hhadfiel@redhat.com>
@hopehadfield
hopehadfield merged commit 8ba3661 into redhat-developer:main Jun 23, 2026
14 checks passed
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants