Skip to content

refactor(plugins): centralize default plugins list - #4079

Merged
EmilyRagan merged 6 commits into
mainfrom
centralize-default-plugins-list
Oct 16, 2026
Merged

EmilyRagan merged 6 commits into
mainfrom
centralize-default-plugins-list

Conversation

@EmilyRagan

@EmilyRagan EmilyRagan commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

What changed

  • The default plugin names now live in one file, openc3/data/config/default_plugins.yaml, read by a new OpenC3::DefaultPlugins module (NAMES, default?).
  • GET /openc3-api/plugins/all flags each plugin with default. The Admin Plugins tab uses that flag instead of a hardcoded list.
  • GET /openc3-api/plugins accepts exclude_default=true to leave out the default plugins. Omitting it, or false, returns every name. Any other value returns 400.
  • Ruby and Python plugin_list pass exclude_default=true unless default is set, so the filtering happens in one place.
  • The duplicate lists in local_mode.rb, local_mode.py (unused) and PluginsTab.vue are removed. plugins.p.spec.ts reads the shared list.

Why it changed

  • The list was copied in four places and the script API filtered by name prefix, which disagreed with the UI's exact match. A tool gem not in the list, such as openc3-cosmos-tool-schedule, was hidden by plugin_list() but shown in the UI.
  • Matching is now by exact short name (the __ suffix and trailing version are stripped), the same rule the UI used.

Testing strategy

  • Added default_plugins_spec.rb, exclude_default and default flag cases in plugins_controller_spec.rb, and reworked the Ruby and Python plugin_list tests to check the request.
  • Ran the controller spec, the Ruby plugin_list, default_plugins and local_mode specs, and the Python plugin and utilities tests.
  • Playwright

Review notes

  • Behavior change: plugin_list() now returns any gem that is not in the YAML, including tool gems with a core prefix.
  • The Admin UI needs the full plugin objects, so it stays on /plugins/all

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features
    • The plugin listing API now identifies default plugins and supports excluding them from results.
    • The Admin plugin list now identifies default plugins using their plugin metadata, and continues to show them when default tools are enabled.
    • Ruby and Python plugin listing calls can request results with default plugins excluded.
  • Bug Fixes
    • Default-plugin identification now consistently accounts for versioned plugin names and suffixes.

@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c828f68e-7a7c-4c6e-ac50-4e58a10110ee
📥 Commits

Reviewing files that changed from the base of the PR and between 3ded1cf and f6f49c3.

📒 Files selected for processing: 14
  • openc3-cosmos-cmd-tlm-api/app/controllers/plugins_controller.rb
  • openc3-cosmos-cmd-tlm-api/spec/controllers/plugins_controller_spec.rb
  • openc3-cosmos-init/plugins/packages/openc3-vue-common/src/tools/admin/tabs/PluginsTab.vue
  • openc3-cosmos-init/plugins/packages/openc3-vue-common/src/tools/admin/tabs/plugins/PluginList.vue
  • openc3/data/config/default_plugins.yaml
  • openc3/lib/openc3/script/plugins.rb
  • openc3/lib/openc3/utilities/default_plugins.rb
  • openc3/lib/openc3/utilities/local_mode.rb
  • openc3/python/openc3/script/plugins.py
  • openc3/python/openc3/utilities/local_mode.py
  • openc3/python/test/script/test_plugins.py
  • openc3/spec/script/plugins_spec.rb
  • openc3/spec/utilities/default_plugins_spec.rb
  • playwright/tests/admin/plugins.p.spec.ts
💤 Files with no reviewable changes: 2
  • openc3-cosmos-init/plugins/packages/openc3-vue-common/src/tools/admin/tabs/PluginsTab.vue
  • openc3/python/openc3/utilities/local_mode.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

📝 Walkthrough

A shared YAML list and Ruby utility now identify default plugins. The API filters plugin names and adds a default flag to plugin records. Ruby and Python helpers request API-side filtering, and the Admin plugin list uses the returned flag.

Changes: Default Plugin Classification

Layer / File(s) Summary
Shared catalog and local mode
openc3/data/config/default_plugins.yaml, openc3/lib/openc3/utilities/default_plugins.rb, openc3/spec/utilities/default_plugins_spec.rb, openc3/lib/openc3/utilities/local_mode.rb, openc3/python/openc3/utilities/local_mode.py
A YAML list supplies default-plugin names. The Ruby utility loads and freezes the names and classifies versioned plugin names. Ruby local mode uses the shared list, and Python local mode removes its local list.
API filtering and plugin flags
openc3-cosmos-cmd-tlm-api/app/controllers/plugins_controller.rb, openc3-cosmos-cmd-tlm-api/spec/controllers/plugins_controller_spec.rb
The controller adds an authorized index action that validates exclude_default and filters default plugins when requested. The all-plugins response adds a default flag to each plugin.
Ruby and Python plugin helpers
openc3/lib/openc3/script/plugins.rb, openc3/python/openc3/script/plugins.py, openc3/spec/script/plugins_spec.rb, openc3/python/test/script/test_plugins.py
Both helpers request API-side exclusion when defaults are not requested and return the API response without local prefix filtering. Specs check requests and returned results.
Admin plugin classification and display
openc3-cosmos-init/plugins/packages/openc3-vue-common/src/tools/admin/tabs/PluginsTab.vue, openc3-cosmos-init/plugins/packages/openc3-vue-common/src/tools/admin/tabs/plugins/PluginList.vue, playwright/tests/admin/plugins.p.spec.ts
The Admin plugin list uses the API-provided default flag and sorts both plugin groups by name. The Playwright test derives expected plugin visibility from the YAML list and edition markers.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant RubyPluginList
  participant PythonPluginList
  participant PluginsController
  participant DefaultPlugins
  RubyPluginList->>PluginsController: Request plugin names with exclude_default=true
  PluginsController->>DefaultPlugins: Check plugin names
  DefaultPlugins-->>PluginsController: Return default-plugin classification
  PluginsController-->>RubyPluginList: Return filtered plugin names
  PythonPluginList->>PluginsController: Request plugin names with exclude_default=true
  PluginsController->>DefaultPlugins: Check plugin names
  DefaultPlugins-->>PluginsController: Return default-plugin classification
  PluginsController-->>PythonPluginList: Return filtered plugin names
Loading

Suggested reviewers: jmthomas

Merge Risk: ⚪ Minimal · up to f6f49

No confirmed issue blocks merging. Default-plugin classification matches the catalog, while unlisted tool gems are intentionally included in plugin listings.

Security architecture risk: 🔵 Low · up to f6f49

The changed endpoints preserve their existing authorization checks and scope handling, and classification does not write plugin state. The main residual risk is inconsistent plugin visibility when API, client, and UI versions are upgraded or rolled back separately.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The changed request parameter only removes names from an existing scoped listing. The new response field classifies records already returned by the all-plugins endpoint; neither addition selects another scope or introduces a storage write.

Trust Boundaries and Controls

  • observed — The explicit index action retains the inherited authorization('system') gate and request-scope forwarding. Show retains the same gate and scoped model access. Default classification is presentation metadata rather than a new authorization decision.
Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 10 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the primary change: centralizing the default plugin list across the API, clients, local mode, and Admin Plugins tab.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 9.09% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 10 files. (2 skipped: 2 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Warning

Some tools did not complete. Review the errors below.

🔧 Brakeman (8.0.6)

Brakeman reported 6 analysis error(s); security analysis is incomplete

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.57%. Comparing base (3ded1cf) to head (f6f49c3).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
...common/src/tools/admin/tabs/plugins/PluginList.vue 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4079      +/-   ##
==========================================
+ Coverage   80.53%   80.57%   +0.03%     
==========================================
  Files         904      905       +1     
  Lines       68992    69004      +12     
  Branches     2681     2679       -2     
==========================================
+ Hits        55565    55601      +36     
+ Misses      12765    12738      -27     
- Partials      662      665       +3     
Flag Coverage Δ
frontend 67.40% <66.66%> (-0.02%) ⬇️
python 80.58% <ø> (+0.05%) ⬆️
ruby-api 83.03% <100.00%> (+0.28%) ⬆️
ruby-backend 86.10% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Base automatically changed from align-plugin_list to main October 16, 2026 15:35
@EmilyRagan
EmilyRagan force-pushed the centralize-default-plugins-list branch from aa56c38 to f6f49c3 Compare October 16, 2026 15:58
@sonarqubecloud

Copy link
Copy Markdown

@EmilyRagan
EmilyRagan marked this pull request as ready for review October 16, 2026 17:22
@EmilyRagan EmilyRagan self-assigned this Oct 16, 2026
@EmilyRagan
EmilyRagan merged commit d963ec2 into main Oct 16, 2026
42 of 44 checks passed
@EmilyRagan
EmilyRagan deleted the centralize-default-plugins-list branch October 16, 2026 21:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants