Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion src/praisonai/praisonai/framework_adapters/registry.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from __future__ import annotations

from typing import Dict, Type, Optional
import inspect
import logging

from .base import FrameworkAdapter
Expand Down Expand Up @@ -62,6 +63,28 @@ def __init__(self) -> None:
entry_point_group="praisonai.framework_adapters",
builtins=_BUILTIN_ADAPTERS
)

def _validate_adapter(self, name: str, adapter) -> None:
"""Validate that adapter implements the required protocol signature."""
_REQUIRED_KW = {"tools_dict", "agent_callback", "task_callback", "cli_config"}

sig = inspect.signature(type(adapter).run)
kw_only = {
p.name for p in sig.parameters.values()
if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD)
}
Comment on lines +72 to +75

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The validation strictly checks for inspect.Parameter.KEYWORD_ONLY. However, parameters defined as POSITIONAL_OR_KEYWORD (i.e., without the * separator in the function signature) can also be safely passed as keyword arguments. This strict check will cause false-positive TypeErrors for perfectly valid custom adapters. Allowing both KEYWORD_ONLY and POSITIONAL_OR_KEYWORD makes the validation more robust.

Suggested change
kw_only = {
p.name for p in sig.parameters.values()
if p.kind is inspect.Parameter.KEYWORD_ONLY
}
kw_only = {
p.name for p in sig.parameters.values()
if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD)
}

missing = _REQUIRED_KW - kw_only
if missing:
raise TypeError(
f"FrameworkAdapter {name!r} does not implement the protocol: "
f"missing keyword-only parameters {sorted(missing)}"
Comment on lines +71 to +80

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Demonstrate that the current inspect predicate accepts positional-or-keyword parameters.
# Expected: current check reports no missing params, while strict keyword-only validation flags all four.

python - <<'PY'
import inspect

_REQUIRED_KW = {"tools_dict", "agent_callback", "task_callback", "cli_config"}

class BadAdapter:
    def run(
        self,
        config,
        llm_config,
        topic,
        tools_dict=None,
        agent_callback=None,
        task_callback=None,
        cli_config=None,
    ):
        pass

sig = inspect.signature(BadAdapter.run)
accepted_by_current_check = {
    p.name
    for p in sig.parameters.values()
    if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD)
}
not_keyword_only = {
    name
    for name in _REQUIRED_KW
    if sig.parameters[name].kind != inspect.Parameter.KEYWORD_ONLY
}

print("missing under current check:", sorted(_REQUIRED_KW - accepted_by_current_check))
print("not keyword-only under strict check:", sorted(not_keyword_only))
PY

Repository: MervinPraison/PraisonAI

Length of output: 294


🏁 Script executed:

cat -n src/praisonai/praisonai/framework_adapters/registry.py | sed -n '60,120p'

Repository: MervinPraison/PraisonAI

Length of output: 2721


🏁 Script executed:

rg "_REQUIRED_KW" src/praisonai/praisonai/framework_adapters/registry.py -A 2 -B 2

Repository: MervinPraison/PraisonAI

Length of output: 651


🏁 Script executed:

rg "class PluginRegistry" src/praisonai/ -A 20

Repository: MervinPraison/PraisonAI

Length of output: 1654


🏁 Script executed:

rg "def create" src/praisonai/praisonai/plugin_base.py -A 15

Repository: MervinPraison/PraisonAI

Length of output: 240


🏁 Script executed:

rg "def create" src/praisonai/praisonai/_registry.py -A 25

Repository: MervinPraison/PraisonAI

Length of output: 1785


🏁 Script executed:

rg "ValueError\|TypeError" src/praisonai/praisonai/_registry.py

Repository: MervinPraison/PraisonAI

Length of output: 161


🏁 Script executed:

rg "def resolve" src/praisonai/praisonai/_registry.py -A 20

Repository: MervinPraison/PraisonAI

Length of output: 757


Enforce the exact keyword-only contract and distinguish protocol validation errors from constructor failures.

Line 74 accepts POSITIONAL_OR_KEYWORD, allowing adapters to pass validation even when their run() method defines parameters as positional-or-keyword instead of keyword-only (as required by the protocol). Additionally, line 111 catches all TypeError broadly, which masks not only protocol validation failures but also legitimate errors from the adapter's constructor—causing them to be silently reported as "unavailable" instead of surfacing the actual error.

Introduce a dedicated AdapterProtocolError exception, enforce strict keyword-only validation, and catch only that exception in is_available().

Proposed fix
+class AdapterProtocolError(TypeError):
+    """Raised when a framework adapter violates the FrameworkAdapter protocol."""
+
+
 class FrameworkAdapterRegistry(PluginRegistry[FrameworkAdapter]):
@@
-        sig = inspect.signature(type(adapter).run)
-        kw_only = {
-            p.name for p in sig.parameters.values()
-            if p.kind in (inspect.Parameter.KEYWORD_ONLY, inspect.Parameter.POSITIONAL_OR_KEYWORD)
-        }
-        missing = _REQUIRED_KW - kw_only
-        if missing:
-            raise TypeError(
+        run = getattr(type(adapter), "run", None)
+        if run is None:
+            raise AdapterProtocolError(
+                f"FrameworkAdapter {name!r} does not implement the protocol: missing run()"
+            )
+
+        sig = inspect.signature(run)
+        params = sig.parameters
+        present = set(params)
+        missing = _REQUIRED_KW - present
+        not_keyword_only = {
+            param_name
+            for param_name in _REQUIRED_KW & present
+            if params[param_name].kind != inspect.Parameter.KEYWORD_ONLY
+        }
+        if missing or not_keyword_only:
+            problems = []
+            if missing:
+                problems.append(f"missing parameters {sorted(missing)}")
+            if not_keyword_only:
+                problems.append(f"not keyword-only parameters {sorted(not_keyword_only)}")
+            raise AdapterProtocolError(
                 f"FrameworkAdapter {name!r} does not implement the protocol: "
-                f"missing keyword-only parameters {sorted(missing)}"
+                + "; ".join(problems)
             )
@@
-        except (ValueError, TypeError):
+        except (ValueError, AdapterProtocolError):
             return False
🤖 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 `@src/praisonai/praisonai/framework_adapters/registry.py` around lines 71 - 80,
Create a new dedicated AdapterProtocolError exception class to distinguish
protocol validation failures from other errors. In the run method signature
validation logic, change the condition to accept only KEYWORD_ONLY parameters
(remove the POSITIONAL_OR_KEYWORD option from the kind check) to enforce the
strict keyword-only contract. Replace the TypeError raised for missing
parameters with the new AdapterProtocolError exception. Finally, update the
exception handling in is_available() to catch only AdapterProtocolError instead
of the broad TypeError catch, allowing legitimate constructor errors to surface
and propagate rather than being silently masked.

)

def create(self, name: str, *args, **kwargs):
"""Create an adapter instance with protocol validation."""
adapter = super().create(name, *args, **kwargs)
self._validate_adapter(name, adapter)
return adapter

# Backward compatibility aliases - delegate to parent methods
def list_registered(self) -> list[str]:
Expand All @@ -85,7 +108,7 @@ def is_available(self, name: str) -> bool:
"""
try:
adapter = self.create(name)
except ValueError:
except (ValueError, TypeError):
return False

try:
Expand Down
Loading