[FIX] Propagate all *_cls/*_class plugin metadata fields in PluginManager#2201
Merged
Conversation
…oded 3 PluginManager.load_plugins() only copied entrypoint_cls, exception_cls, and service_class from a plugin's metadata into the registered plugin dict. The verticals_usage plugin (which feeds API deployment usage into the API Hub subscription dashboard) also defines headers_cache_class, which was silently dropped, causing every plugin["headers_cache_class"] lookup in api_hub_usage_utils.py to raise KeyError - swallowed by broad except blocks - so API hub headers were never cached and usage never reached the verticals.subscription_usage table.
Contributor
Summary by CodeRabbit
Walkthrough
ChangesPlugin metadata loading
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
|
| Filename | Overview |
|---|---|
| unstract/core/src/unstract/core/plugins/plugin_manager.py | Replaces the fixed class-handle allowlist with suffix-based metadata promotion. |
| unstract/core/tests/test_plugin_manager.py | Adds focused coverage for metadata propagation and plugin activation filtering. |
Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/plugin-mana..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@unstract/core/src/unstract/core/plugins/plugin_manager.py`:
- Around line 218-224: Update the metadata iteration in the plugin loading logic
to validate that each key is a string before applying the "_cls"/"_class" suffix
check. Skip non-string keys while preserving the existing condition that only
adds missing plugin_data fields.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f3a67017-5865-495c-8dc9-587afd840ffe
📒 Files selected for processing (2)
unstract/core/src/unstract/core/plugins/plugin_manager.pyunstract/core/tests/test_plugin_manager.py
jaseemjaskp
commented
Jul 23, 2026
jaseemjaskp
left a comment
jaseemjaskp
left a comment
Contributor
Author
There was a problem hiding this comment.
Automated PR review (PR Review Toolkit: code-reviewer, silent-failure-hunter, type-design-analyzer, pr-test-analyzer, comment-analyzer, code-simplifier). The core fix is correct and well-scoped; these are refinements. The non-string-key AttributeError case is already covered by CodeRabbit's comment and is not repeated here.
…rage - key.endswith() on a non-string metadata key raised AttributeError, uncaught by the surrounding except KeyError, crashing load_plugins() for every plugin process-wide (CodeRabbit). - Hoist the _cls/_class suffix tuple to a module constant and trim the comment so it doesn't reference one enterprise plugin's field name. - Note that the `key not in plugin_data` guard is purely defensive today. - Add tests locking in that a *_class field propagates by name even when its value is None, and that is_active=False also skips loading.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
unstract/core/tests/test_plugin_manager.py (1)
42-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression test for non-string metadata keys.
The loader now skips non-string keys before checking
_cls/_class, but this test does not pin that protection. Add an integer-key metadata entry and assert the plugin still loads successfully.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@unstract/core/tests/test_plugin_manager.py` around lines 42 - 58, Add an integer-key metadata entry to the plugin fixture in test_known_and_unknown_class_fields_are_both_registered, while retaining the existing class-field assertions, and assert the plugin loads successfully without error. Ensure the non-string key is ignored safely during metadata processing and does not affect the existing plugin name or registered class fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@unstract/core/tests/test_plugin_manager.py`:
- Around line 42-58: Add an integer-key metadata entry to the plugin fixture in
test_known_and_unknown_class_fields_are_both_registered, while retaining the
existing class-field assertions, and assert the plugin loads successfully
without error. Ensure the non-string key is ignored safely during metadata
processing and does not affect the existing plugin name or registered class
fields.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f90c5343-010e-4a47-99b5-9e5bf4a9b427
📒 Files selected for processing (2)
unstract/core/src/unstract/core/plugins/plugin_manager.pyunstract/core/tests/test_plugin_manager.py
🚧 Files skipped from review as they are similar to previous changes (1)
- unstract/core/src/unstract/core/plugins/plugin_manager.py
chandrasekharan-zipstack
approved these changes
Jul 24, 2026
athul-rs
approved these changes
Jul 24, 2026
|
Contributor
Unstract test resultsPer-group results
Critical paths
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
PluginManager.load_plugins()(inunstract/core) only copied a hardcoded 3-field allowlist (entrypoint_cls,exception_cls,service_class) from a plugin'smetadatadict into the registered plugin dict._clsor_class, so new plugin fields don't require this shared loader to be updated every time.Why
verticals_usageplugin (enterprise/cloud-only, feeds API deployment usage into the API Hub subscription dashboard) defines aheaders_cache_classmetadata field that the loader's hardcoded allowlist didn't know about. It was silently dropped, so everyplugin["headers_cache_class"]lookup inbackend/plugins/workflow_manager/workflow_v2/api_hub_usage_utils.pyraisedKeyError— swallowed by broadexcept Exceptionblocks.store_usage()(the only path that writes to theverticals.subscription_usagetable) was never reached, and the API Hub portal's subscription dashboard stopped showing usage — with no visible errors anywhere.api_hub_usage_utils.pyfrom a build-time file-replacement model to a runtimeget_plugin()dict lookup, without the sharedPluginManager(added in UN-2930 [MISC] Add plugin infrastructure to unstract-core #1618) being updated to support theheaders_cache_classfield.How
if "x" in metadata: plugin_data["x"] = metadata["x"]checks inunstract/core/src/unstract/core/plugins/plugin_manager.pywith a loop that copies any metadata key matching*_cls/*_class.unstract/core/tests/test_plugin_manager.pycovering: unknown_class/_clsfields (e.g.headers_cache_class) now propagate, existingentrypoint_cls/exception_clsstill work, non-class metadata isn't duplicated at the top level, and disabled plugins are still skipped.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
backend/that no other plugin's metadata relies on a field outside the previous three-field allowlist (onlyservice_classis used elsewhere — notifications, hubspot, subscription_usage, file_converter, payload_modifier, etc.), so existing plugin behavior is unchanged; this purely adds support for previously-dropped fields.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
unstract/core/tests/test_plugin_manager.py(4 new tests, all passing).unstract/coretest suite (uv run pytest unstract/core/tests/) — no new regressions; 2 pre-existing unrelated failures were already broken before this change.ruff check/ruff format --checkon changed files — clean.Screenshots
Checklist
I have read and understood the Contribution Guidelines.