refactor(plugins): decouple plugin framework data models#2895
refactor(plugins): decouple plugin framework data models#2895
Conversation
crivetimihai
left a comment
There was a problem hiding this comment.
Thanks for this substantial and well-structured decoupling work, @araujof. The protocol-based type contracts and hook payload policies are clean architectural improvements. A few items:
Bug: LazySettingsWrapper.enabled ignores .env / pydantic-settings resolution
settings.py:~1962-1971: When PLUGINS_ENABLED is absent from os.environ, the property hard-returns False instead of delegating to get_settings().enabled. If PLUGINS_ENABLED=true is set in a .env file (which pydantic-settings would parse), it gets silently ignored. Same issue as noted in PR #2829 — consider falling back to get_settings().enabled.
Bug: regex_filter/search_replace.py dict branch still mutates frozen payload in-place
Lines 139-144 (unchanged) still do payload.result[key] = value inside a loop. While frozen=True only guards attribute reassignment, this bypasses the model_copy-based modification pattern and won't produce a properly tracked modified_payload. The str branch was correctly fixed; the dict branch needs the same treatment.
Duplicated TransportType enum
models.py now defines its own TransportType identical to common/models.py. This risks divergence. Consider re-exporting via a thin alias or adding a test asserting both enums have the same members.
lru_cache on get_settings() prevents test isolation
settings.py:1934: @lru_cache() means env var changes in tests won't take effect after the first call. Consider exposing get_settings.cache_clear().
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
e45a2d9 to
ef1677c
Compare
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
ef1677c to
a57e7df
Compare
Signed-off-by: Frederico Araujo <frederico.araujo@ibm.com>
Summary of changes:
|
Summary
Eliminates reverse dependencies from the plugin framework (
mcpgateway/plugins/framework/) back into the outer gateway package (mcpgateway.common,mcpgateway.utils), and introduces hook payload policies for controlled modification of plugin payloads.After this change, the plugin framework has zero imports from
mcpgateway.commonormcpgateway.utils, making it viable as a standalone, extractable package.This PR depends on #2829 (
refactor/plugins_settings).Closes: #2859
Changes
Protocol definitions for boundary types
protocols.pywith@runtime_checkableMessageLikeandPromptResultLikeprotocolsMessageandPromptResultfrommcpgateway.common.modelsImmutable payloads
PluginPayloadfrom a bareTypeAlias = BaseModelto a proper frozen base class (ConfigDict(frozen=True, arbitrary_types_allowed=True))model_copy(update={...})instead of in-place mutationInternal framework types
TransportTypeenum internally inmodels.py(was imported frommcpgateway.common.models)SecurityValidatorinvalidators.pywith only the two methods used by the framework (validate_url, path validation)ORJSONResponseintoutils.py(was imported frommcpgateway.utils.orjson_response)Hook payload policies
hooks/policies.pywithHookPayloadPolicy,DefaultHookPolicy, andapply_policy()— the framework provides the mechanismmcpgateway/plugins/policy.py— the gateway defines the concrete policies per hook type (tool, prompt, resource, agent)PluginManagervia dependency injection, keeping the framework decoupled from gateway-specific decisionsdefault_hook_policysetting (PLUGINS_DEFAULT_HOOK_POLICYenv var) controls behavior for hooks without explicit policies:allow(default, backwards compatible) ordeny(strict mode)Policy enforcement in executor
PluginExecutorapplies per-plugin policy filtering after each plugin returnswritable_fieldsare accepted; all other modifications are silently reverted to the original valuesdefault_hook_policyJSON deserialization for
Any-typed fieldsStructuredDatamodel andcoerce_nested()utility for recursive dict-to-object conversionPromptPosthookPayload.resultso that external server flows (gRPC, MCP stdio, Unix socket, streamable HTTP) correctly reconstruct nested objects with attribute accessPlugin updates for frozen payloads
model_copy(update={...})instead of direct attribute assignmentHeadersPluginto use the same patternFiles changed
framework/protocols.pyMessageLike,PromptResultLikeprotocol classesframework/validators.pySecurityValidatorsubsetframework/hooks/policies.pyHookPayloadPolicy,DefaultHookPolicy,apply_policyplugins/policy.pyHOOK_PAYLOAD_POLICIESdictionarytests/.../test_policies.pyframework/models.pyTransportTypeenum, frozenPluginPayloadbase classframework/hooks/agents.pyAny+ protocol docs instead ofMessageimportframework/hooks/prompts.pyAny+ field validator instead ofPromptResultimportframework/hooks/registry.pyframework/manager.pyhook_policiesvia DI, enforce in executor loopframework/settings.pydefault_hook_policyfieldframework/utils.pyStructuredData,coerce_nested,ORJSONResponseframework/__init__.pyHOOK_PAYLOAD_POLICIESwhen creating managerframework/external/mcp/client.pyTransportTypefrom frameworkframework/external/mcp/server/runtime.pyORJSONResponsefrom frameworkplugins/vault/vault_plugin.pymodel_copyfor frozen payload compatibilityplugins/regex_filter/search_replace.pymodel_copyfor frozen payload compatibilityplugins/pii_filter/pii_filter.pymodel_copyfor frozen payload compatibilityplugins/external/cedar/.../plugin.pymodel_copyfor frozen payload compatibilitytests/.../headers.pymodel_copyfor frozen payload compatibilityNote to maintainer
Please merge #2829 first.