diff --git a/features/actor_options_propagation.feature b/features/actor_options_propagation.feature new file mode 100644 index 000000000..cdf232b59 --- /dev/null +++ b/features/actor_options_propagation.feature @@ -0,0 +1,109 @@ +Feature: Actor options propagation to LLM constructor + Tests that actor-level `options` (openai_api_base, openai_api_key, etc.) + are correctly forwarded to `ProviderRegistry.create_llm()` across all + call sites: Strategize phase, Execute phase, and SessionWorkflow. + + Forgejo: #11256 + + # ------------------------------------------------------------------ + # build_llm_kwargs_from_options shared utility + # ------------------------------------------------------------------ + + Scenario: build_llm_kwargs_from_options forwards openai_api_base + Given aop actor options with "openai_api_base" set to "http://localhost:8000/v1" + When I call build_llm_kwargs_from_options + Then the result should contain "openai_api_base" with value "http://localhost:8000/v1" + + Scenario: build_llm_kwargs_from_options routes openai_api_key through sentinel + Given aop actor options with "openai_api_key" set to "none" + When I call build_llm_kwargs_from_options + Then the result should contain "__api_key_sentinel" with value "none" + And the result should not contain "openai_api_key" + + Scenario: build_llm_kwargs_from_options forwards allowed temperature + Given aop actor options with "temperature" set to "0.7" + When I call build_llm_kwargs_from_options + Then the result should contain "temperature" with value 0.7 + + Scenario: build_llm_kwargs_from_options rejects reserved keys + Given aop actor options with "provider_type" set to "anthropic" + When I call build_llm_kwargs_from_options + Then the result should not contain "provider_type" + + Scenario: build_llm_kwargs_from_options rejects unknown keys + Given aop actor options with "dangerous_param" set to "value" + When I call build_llm_kwargs_from_options + Then the result should not contain "dangerous_param" + + Scenario: build_llm_kwargs_from_options with None returns empty dict + Given aop actor options is None + When I call build_llm_kwargs_from_options + Then the result should be an empty dict + + Scenario: build_llm_kwargs_from_options with empty dict returns empty dict + Given aop actor options is an empty dict + When I call build_llm_kwargs_from_options + Then the result should be an empty dict + + Scenario: build_llm_kwargs_from_options forwards multiple allowed keys + Given aop actor options with "openai_api_base" set to "http://local:8000/v1" + And aop actor options with "max_tokens" set to "2048" + And aop actor options with "timeout" set to "60" + When I call build_llm_kwargs_from_options + Then the result should contain "openai_api_base" with value "http://local:8000/v1" + And the result should contain "max_tokens" with value 2048 + And the result should contain "timeout" with value 60 + + # ------------------------------------------------------------------ + # StrategyActor forwards actor options to create_llm + # ------------------------------------------------------------------ + +Scenario: StrategyActor resolves actor options and forwards to create_llm + Given aop a mock lifecycle service that resolves actor "local/test-strategist" + And aop the resolved actor options contain "openai_api_base" set to "http://backend:9000/v1" + And aop the resolved actor options contain "openai_api_key" set to "test-key" + And aop a mock provider registry that captures create_llm kwargs with LLM response + When aop the StrategyActor executes with LLM for plan "01KSQ4ADB3AVXTWEK2DD0ZJDKN" + Then aop create_llm should have been called + And aop create_llm kwargs should contain "openai_api_base" with value "http://backend:9000/v1" + And aop create_llm kwargs should contain "__api_key_sentinel" with value "test-key" + + Scenario: StrategyActor creates LLM without options when actor has no options + Given aop a mock lifecycle service that resolves actor "openai/gpt-4" + And aop the resolved actor options is None + And aop a mock provider registry that captures create_llm kwargs with LLM response + When aop the StrategyActor executes with LLM for plan "01KSQ4ADB3AVXTWEK2DD0ZJCKN" + Then aop create_llm should have been called + And aop create_llm kwargs should not contain "openai_api_base" + + # ------------------------------------------------------------------ + # LLMStrategizeActor forwards actor options to create_llm + # ------------------------------------------------------------------ + + Scenario: LLMStrategizeActor forwards actor options to create_llm + Given aop a valid LLMStrategizeActor with actor options + When aop I call strategize execute with plan_id "PLAN_OPT" and stream callback + Then aop the strategize create_llm should have received "openai_api_base" with value "http://custom-backend:7000/v1" + And aop the strategize create_llm should have received "__api_key_sentinel" with value "local-key" + + Scenario: LLMStrategizeActor creates LLM without options when none exist + Given aop a valid LLMStrategizeActor without actor options + When aop I call strategize execute with plan_id "PLAN_NOOPT" and no stream callback + Then aop the strategize create_llm should not have received "openai_api_base" + And aop the strategize create_llm should not have received "__api_key_sentinel" + + # ------------------------------------------------------------------ + # LLMExecuteActor forwards actor options to create_llm + # ------------------------------------------------------------------ + + Scenario: LLMExecuteActor forwards actor options to create_llm + Given aop a valid LLMExecuteActor with actor options + When aop I call execute actor with plan_id "EXEC_OPT" and read_only False + Then aop the execute create_llm should have received "openai_api_base" with value "http://custom-executor:9000/v1" + And aop the execute create_llm should have received "__api_key_sentinel" with value "exec-key" + + Scenario: LLMExecuteActor creates LLM without options when none exist + Given aop a valid LLMExecuteActor without actor options + When aop I call execute actor with plan_id "EXEC_NOOPT" and read_only False + Then aop the execute create_llm should not have received "openai_api_base" + And aop the execute create_llm should not have received "__api_key_sentinel" \ No newline at end of file diff --git a/features/mocks/mock_strategy_llm.py b/features/mocks/mock_strategy_llm.py index e3a91b1fc..308254390 100644 --- a/features/mocks/mock_strategy_llm.py +++ b/features/mocks/mock_strategy_llm.py @@ -321,6 +321,7 @@ def make_mock_lifecycle( get_plan=MagicMock(return_value=plan), get_action=MagicMock(return_value=action), resolve_actor_provider_model=MagicMock(return_value=None), + resolve_actor_options=MagicMock(return_value=None), ) diff --git a/features/steps/actor_options_propagation_steps.py b/features/steps/actor_options_propagation_steps.py new file mode 100644 index 000000000..21c3b82e0 --- /dev/null +++ b/features/steps/actor_options_propagation_steps.py @@ -0,0 +1,386 @@ +"""Step definitions for actor_options_propagation.feature. + +Tests that actor-level ``options`` (openai_api_base, openai_api_key, +etc.) are correctly forwarded to ``ProviderRegistry.create_llm()`` across +all call sites: Strategize phase, Execute phase, and SessionWorkflow. + +All step text uses the ``aop`` prefix to avoid collisions with other +step files that manipulate ``context.mock_lifecycle`` etc. + +Forgejo: #11256 +""" + +from types import SimpleNamespace +from unittest.mock import MagicMock + +from behave import given, then, when + +from cleveragents.actor.config import build_llm_kwargs_from_options +from cleveragents.application.services.llm_actors import ( + LLMExecuteActor, + LLMStrategizeActor, +) +from cleveragents.application.services.strategy_actor import StrategyActor +from cleveragents.core.exceptions import ValidationError + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _aop_make_plan(action_name="test-action"): + return SimpleNamespace(action_name=action_name) + + +def _aop_make_action(strategy_actor=None, execution_actor=None): + return SimpleNamespace( + strategy_actor=strategy_actor, + execution_actor=execution_actor, + ) + + +def _aop_llm_response(content): + return SimpleNamespace(content=content) + + +def _aop_capturing_registry(): + """Create a MagicMock registry that captures create_llm kwargs.""" + llm = MagicMock() + llm.invoke.return_value = SimpleNamespace(content="[]") + captured = {} + + def _fake_create(provider_type=None, model_id=None, **kw): + captured["provider_type"] = provider_type + captured["model_id"] = model_id + captured["kwargs"] = kw + return llm + + reg = MagicMock() + reg.create_llm.side_effect = _fake_create + return reg, captured + + +# --------------------------------------------------------------------------- +# build_llm_kwargs_from_options shared utility +# --------------------------------------------------------------------------- + + +@given('aop actor options with "{key}" set to "{value}"') +def step_aop_given_options_with_key(context, key, value): + if not hasattr(context, "aop_options"): + context.aop_options = {} + try: + context.aop_options[key] = int(value) + except ValueError: + try: + context.aop_options[key] = float(value) + except ValueError: + context.aop_options[key] = value + + +@given("aop actor options is None") +def step_aop_given_options_none(context): + context.aop_options = None + + +@given("aop actor options is an empty dict") +def step_aop_given_options_empty(context): + context.aop_options = {} + + +@when("I call build_llm_kwargs_from_options") +def step_aop_when_build_llm_kwargs(context): + context.aop_result = build_llm_kwargs_from_options(context.aop_options) + + +@then('the result should contain "{key}" with value "{value}"') +def step_aop_then_contains_str(context, key, value): + kwargs = context.aop_result + assert key in kwargs, f"Expected '{key}' in result, got {kwargs}" + assert kwargs[key] == value, f"Expected {key}={value}, got {kwargs[key]}" + + +@then('the result should contain "{key}" with value {value}') +def step_aop_then_contains_int(context, key, value): + kwargs = context.aop_result + assert key in kwargs, f"Expected '{key}' in result, got {kwargs}" + try: + expected_val = int(value) + except ValueError: + expected_val = float(value) + assert kwargs[key] == expected_val, ( + f"Expected {key}={expected_val}, got {kwargs[key]}" + ) + + +@then('the result should not contain "{key}"') +def step_aop_then_not_contains(context, key): + assert key not in context.aop_result, ( + f"Expected '{key}' NOT in result, got {context.aop_result}" + ) + + +@then("the result should be an empty dict") +def step_aop_then_empty_dict(context): + assert context.aop_result == {}, f"Expected empty dict, got {context.aop_result}" + + +# --------------------------------------------------------------------------- +# StrategyActor options propagation +# --------------------------------------------------------------------------- + + +@given('aop a mock lifecycle service that resolves actor "{actor_name}"') +def step_aop_given_lifecycle(context, actor_name): + plan = _aop_make_plan() + action = _aop_make_action(strategy_actor=actor_name) + context.aop_lifecycle = SimpleNamespace( + get_plan=MagicMock(return_value=plan), + get_action=MagicMock(return_value=action), + resolve_actor_provider_model=MagicMock(return_value="openai/gpt-4"), + resolve_actor_options=MagicMock(return_value=None), + ) + + +@given('aop the resolved actor options contain "{key}" set to "{value}"') +def step_aop_given_options_contain(context, key, value): + cur = context.aop_lifecycle.resolve_actor_options.return_value or {} + cur[key] = value + context.aop_lifecycle.resolve_actor_options.return_value = cur + + +@given("aop the resolved actor options is None") +def step_aop_given_options_is_none(context): + context.aop_lifecycle.resolve_actor_options.return_value = None + + +@given("aop a mock provider registry that captures create_llm kwargs with LLM response") +def step_aop_given_capturing_registry(context): + reg, captured = _aop_capturing_registry() + context.aop_registry = reg + context.aop_captured_kwargs = captured + + +@when('aop the StrategyActor executes with LLM for plan "{plan_id}"') +def step_aop_when_strategy_actor_executes(context, plan_id): + actor = StrategyActor( + provider_registry=context.aop_registry, + lifecycle_service=context.aop_lifecycle, + ) + import contextlib + + with contextlib.suppress( + ValidationError, AttributeError, KeyError, TypeError, ValueError + ): + actor.execute( + plan_id=plan_id, + definition_of_done="test", + resources=None, + project_context=None, + invariants=None, + ) + + +@then("aop create_llm should have been called") +def step_aop_then_create_llm_called(context): + assert context.aop_registry.create_llm.called, ( + "Expected create_llm to have been called" + ) + + +@then('aop create_llm kwargs should contain "{key}" with value "{value}"') +def step_aop_then_llm_kwargs_contain(context, key, value): + kwargs = context.aop_captured_kwargs.get("kwargs", {}) + assert key in kwargs, f"Expected '{key}' in create_llm kwargs, got {kwargs}" + assert kwargs[key] == value, f"Expected {key}={value}, got {kwargs[key]}" + + +@then('aop create_llm kwargs should not contain "{key}"') +def step_aop_then_llm_kwargs_not_contain(context, key): + kwargs = context.aop_captured_kwargs.get("kwargs", {}) + assert key not in kwargs, f"Expected '{key}' NOT in create_llm kwargs, got {kwargs}" + + +# --------------------------------------------------------------------------- +# LLMStrategizeActor options propagation +# --------------------------------------------------------------------------- + + +@given("aop a valid LLMStrategizeActor with actor options") +def step_aop_given_strategize_with_options(context): + reg, captured = _aop_capturing_registry() + context.aop_registry = reg + context.aop_captured_kwargs = captured + plan = _aop_make_plan() + action = _aop_make_action(strategy_actor="local/test-backend") + context.aop_lifecycle = SimpleNamespace( + get_plan=MagicMock(return_value=plan), + get_action=MagicMock(return_value=action), + resolve_actor_provider_model=MagicMock(return_value="openai/gpt-4"), + resolve_actor_options=MagicMock( + return_value={ + "openai_api_base": "http://custom-backend:7000/v1", + "openai_api_key": "local-key", + } + ), + ) + context.aop_strategize_actor = LLMStrategizeActor( + provider_registry=context.aop_registry, + lifecycle_service=context.aop_lifecycle, + ) + + +@given("aop a valid LLMStrategizeActor without actor options") +def step_aop_given_strategize_without_options(context): + reg, captured = _aop_capturing_registry() + context.aop_registry = reg + context.aop_captured_kwargs = captured + plan = _aop_make_plan() + action = _aop_make_action(strategy_actor="openai/gpt-4") + context.aop_lifecycle = SimpleNamespace( + get_plan=MagicMock(return_value=plan), + get_action=MagicMock(return_value=action), + resolve_actor_provider_model=MagicMock(return_value="openai/gpt-4"), + resolve_actor_options=MagicMock(return_value=None), + ) + context.aop_strategize_actor = LLMStrategizeActor( + provider_registry=context.aop_registry, + lifecycle_service=context.aop_lifecycle, + ) + + +@when("aop I call strategize execute with plan_id ") +@when('aop I call strategize execute with plan_id "{plan_id}" and stream callback') +def step_aop_when_strategize_execute(context, plan_id="PLAN_OPT"): + events = [] + + def callback(event_type, data): + events.append({"type": event_type, "data": data}) + + context.aop_strategize_result = context.aop_strategize_actor.execute( + plan_id=plan_id, + definition_of_done="Build a REST API", + stream_callback=callback, + ) + + +@when('aop I call strategize execute with plan_id "{plan_id}" and no stream callback') +def step_aop_when_strategize_execute_no_cb(context, plan_id): + context.aop_strategize_result = context.aop_strategize_actor.execute( + plan_id=plan_id, + definition_of_done="Build a REST API", + stream_callback=None, + ) + + +@then('aop the strategize create_llm should have received "{key}" with value "{value}"') +def step_aop_then_strategize_received(context, key, value): + kwargs = context.aop_captured_kwargs.get("kwargs", {}) + assert key in kwargs, ( + f"Expected '{key}' in strategize create_llm kwargs, got {kwargs}" + ) + assert kwargs[key] == value, f"Expected {key}={value}, got {kwargs[key]}" + + +@then('aop the strategize create_llm should not have received "{key}"') +def step_aop_then_strategize_not_received(context, key): + kwargs = context.aop_captured_kwargs.get("kwargs", {}) + assert key not in kwargs, ( + f"Expected '{key}' NOT in strategize create_llm kwargs, got {kwargs}" + ) + + +# --------------------------------------------------------------------------- +# LLMExecuteActor options propagation +# --------------------------------------------------------------------------- + + +def _aop_executing_registry(content): + """Create a MagicMock registry + lifecycle for LLMExecuteActor tests.""" + llm_resp = _aop_llm_response(content) + llm = MagicMock() + llm.invoke.return_value = llm_resp + captured = {} + + def _fake_create(provider_type=None, model_id=None, **kw): + captured["provider_type"] = provider_type + captured["model_id"] = model_id + captured["kwargs"] = kw + return llm + + reg = MagicMock() + reg.create_llm.side_effect = _fake_create + return reg, captured + + +@given("aop a valid LLMExecuteActor with actor options") +def step_aop_given_execute_with_options(context): + reg, captured = _aop_executing_registry("mock content") + context.aop_registry = reg + context.aop_captured_kwargs = captured + plan = _aop_make_plan() + action = _aop_make_action(execution_actor="local/test-executor") + context.aop_lifecycle = SimpleNamespace( + get_plan=MagicMock(return_value=plan), + get_action=MagicMock(return_value=action), + resolve_actor_provider_model=MagicMock(return_value="openai/gpt-4"), + resolve_actor_options=MagicMock( + return_value={ + "openai_api_base": "http://custom-executor:9000/v1", + "openai_api_key": "exec-key", + } + ), + ) + context.aop_execute_actor = LLMExecuteActor( + provider_registry=context.aop_registry, + lifecycle_service=context.aop_lifecycle, + ) + + +@given("aop a valid LLMExecuteActor without actor options") +def step_aop_given_execute_without_options(context): + reg, captured = _aop_executing_registry("mock content") + context.aop_registry = reg + context.aop_captured_kwargs = captured + plan = _aop_make_plan() + action = _aop_make_action(execution_actor="openai/gpt-4") + context.aop_lifecycle = SimpleNamespace( + get_plan=MagicMock(return_value=plan), + get_action=MagicMock(return_value=action), + resolve_actor_provider_model=MagicMock(return_value="openai/gpt-4"), + resolve_actor_options=MagicMock(return_value=None), + ) + context.aop_execute_actor = LLMExecuteActor( + provider_registry=context.aop_registry, + lifecycle_service=context.aop_lifecycle, + ) + + +@when('aop I call execute actor with plan_id "{plan_id}" and read_only {read_only}') +def step_aop_when_execute_actor(context, plan_id, read_only): + ro = read_only in ("True", "true") + try: + context.aop_execute_result = context.aop_execute_actor.execute( + plan_id=plan_id, + decisions=[], + sandbox_root=None, + stream_callback=None, + read_only=ro, + ) + except (ValidationError, ValueError, RuntimeError): + context.aop_execute_result = None + + +@then('aop the execute create_llm should have received "{key}" with value "{value}"') +def step_aop_then_execute_received(context, key, value): + kwargs = context.aop_captured_kwargs.get("kwargs", {}) + assert key in kwargs, f"Expected '{key}' in execute create_llm kwargs, got {kwargs}" + assert kwargs[key] == value, f"Expected {key}={value}, got {kwargs[key]}" + + +@then('aop the execute create_llm should not have received "{key}"') +def step_aop_then_execute_not_received(context, key): + kwargs = context.aop_captured_kwargs.get("kwargs", {}) + assert key not in kwargs, ( + f"Expected '{key}' NOT in execute create_llm kwargs, got {kwargs}" + ) diff --git a/features/steps/llm_actors_coverage_steps.py b/features/steps/llm_actors_coverage_steps.py index 8142fdd45..9ac8d2ec1 100644 --- a/features/steps/llm_actors_coverage_steps.py +++ b/features/steps/llm_actors_coverage_steps.py @@ -93,6 +93,7 @@ def _make_mock_lifecycle(strategy_actor=None, execution_actor=None): get_plan=MagicMock(return_value=plan), get_action=MagicMock(return_value=action), resolve_actor_provider_model=MagicMock(return_value=None), + resolve_actor_options=MagicMock(return_value=None), ) return lifecycle diff --git a/robot/helper_m5_e2e_context.py b/robot/helper_m5_e2e_context.py index 2e29b2079..b1cd2750f 100644 --- a/robot/helper_m5_e2e_context.py +++ b/robot/helper_m5_e2e_context.py @@ -476,6 +476,7 @@ def llm_execute_acms_context() -> None: get_plan=MagicMock(return_value=plan), get_action=MagicMock(return_value=action), resolve_actor_provider_model=MagicMock(return_value=None), + resolve_actor_options=MagicMock(return_value=None), ) actor = LLMExecuteActor( diff --git a/src/cleveragents/a2a/facade.py b/src/cleveragents/a2a/facade.py index 268a543b9..915d43032 100644 --- a/src/cleveragents/a2a/facade.py +++ b/src/cleveragents/a2a/facade.py @@ -384,10 +384,14 @@ class A2aLocalFacade: raise RuntimeError("Session service not available — create a session first") registry = self._provider_registry actor_resolver = self._build_actor_resolver_for_session_workflow() + actor_options_resolver = ( + self._build_actor_options_resolver_for_session_workflow() + ) return SessionWorkflow( session_service=svc, provider_registry=registry, actor_resolver=actor_resolver, + actor_options_resolver=actor_options_resolver, ) @staticmethod @@ -413,6 +417,33 @@ class A2aLocalFacade: ) return None + @staticmethod + def _build_actor_options_resolver_for_session_workflow(): + """Build a namespace/name -> options dict resolver for SessionWorkflow. + + Returns a callable ``(actor_name: str) -> dict | None``, or + ``None`` when the DI container or actor service is unavailable. + """ + try: + from cleveragents.application.container import get_container + + container = get_container() + actor_service = container.actor_service() + if actor_service is None: + return None + + from cleveragents.application.services.strategy_resolution import ( + build_actor_options_resolver, + ) + + return build_actor_options_resolver(actor_service) + except Exception: + logger.warning( + "actor_options_resolver_unavailable", + exc_info=True, + ) + return None + def _handle_message_send(self, params: dict[str, Any]) -> dict[str, Any]: """Handle A2A ``message/send`` — invoke orchestrator actor (non-streaming). diff --git a/src/cleveragents/actor/config.py b/src/cleveragents/actor/config.py index 014af336b..6e58679d2 100644 --- a/src/cleveragents/actor/config.py +++ b/src/cleveragents/actor/config.py @@ -47,6 +47,73 @@ def _parse_combined_actor_field(data: dict[str, Any]) -> tuple[str | None, str | return None, None +_ALLOWED_LLM_OPTIONS: frozenset[str] = frozenset( + { + "openai_api_base", + "temperature", + "max_tokens", + "timeout", + "top_p", + "frequency_penalty", + "presence_penalty", + } +) + +_RESERVED_LLM_OPTIONS: frozenset[str] = frozenset({"provider_type", "model_id"}) + + +def build_llm_kwargs_from_options( + options: dict[str, Any] | None, + *, + logger: Any = None, +) -> dict[str, Any]: + """Convert actor ``options`` into kwargs for :meth:`ProviderRegistry.create_llm`. + + Actors may declare an ``options`` block in their YAML configuration + containing LLM constructor arguments such as ``openai_api_base`` and + ``openai_api_key`` for custom/local backends. This function extracts + the permitted keys and applies the ``__api_key_sentinel`` pattern so + that explicit actor-level API keys take precedence over + environment/registry defaults. + + Args: + options: Raw options dict from the actor config blob (may be + ``None`` or empty). + logger: Optional structlog-style logger for unrecognised/reserved + key warnings. + + Returns: + Merged kwargs dict suitable for ``**`` unpacking into + ``create_llm()``. Returns an empty dict when *options* is + ``None`` or empty. + """ + if not options: + return {} + kwargs: dict[str, Any] = {} + opts = dict(options) + if "openai_api_key" in opts: + kwargs["__api_key_sentinel"] = opts.pop("openai_api_key") + for key, value in opts.items(): + if key in _RESERVED_LLM_OPTIONS: + if logger is not None: + logger.warning( + "Actor options block contains reserved key '%s' " + "that conflicts with positional arguments; ignoring.", + key, + ) + continue + if key in _ALLOWED_LLM_OPTIONS: + kwargs[key] = value + elif logger is not None: + logger.warning( + "Actor options block contains unrecognized key '%s'; " + "ignoring for security. Allowed keys: %s.", + key, + sorted(_ALLOWED_LLM_OPTIONS), + ) + return kwargs + + class ActorConfiguration(BaseModel): """Canonical actor configuration parsed from user-provided blobs.""" diff --git a/src/cleveragents/actor/schema.py b/src/cleveragents/actor/schema.py index 865a79c86..a1b515c6c 100644 --- a/src/cleveragents/actor/schema.py +++ b/src/cleveragents/actor/schema.py @@ -807,6 +807,12 @@ class ActorConfigSchema(BaseModel): default_factory=dict, description="Environment variable mappings" ) + # Opaque actor options forwarded to LLM constructor (openai_api_base, etc.) + options: dict[str, Any] = Field( + default_factory=dict, + description="Opaque actor options (openai_api_base, openai_api_key, etc.)", + ) + @field_validator("name") @classmethod def validate_name(cls, v: str) -> str: diff --git a/src/cleveragents/application/services/llm_actors.py b/src/cleveragents/application/services/llm_actors.py index 07f114e87..479e9a47b 100644 --- a/src/cleveragents/application/services/llm_actors.py +++ b/src/cleveragents/application/services/llm_actors.py @@ -70,6 +70,10 @@ class PlanLifecycleProtocol(Protocol): """Resolve a namespaced actor name to ``provider/model`` format.""" ... + def resolve_actor_options(self, actor_name: str) -> dict[str, Any] | None: + """Resolve a namespaced actor name to its opaque options dict.""" + ... + # --------------------------------------------------------------------------- # Internal data structures @@ -167,6 +171,11 @@ class LLMStrategizeActor: action = self._lifecycle.get_action(plan.action_name) actor_name = action.strategy_actor or "openai/gpt-4" + # Resolve actor options BEFORE the name is resolved to + # provider/model (because the original namespace/name is + # needed for options lookup). + actor_options = self._lifecycle.resolve_actor_options(actor_name) or {} + # Pre-resolve namespace/name to provider/model via actor registry resolved = self._lifecycle.resolve_actor_provider_model(actor_name) if resolved: @@ -182,7 +191,12 @@ class LLMStrategizeActor: model=model_id, ) - llm = self._registry.create_llm(provider_type=provider_type, model_id=model_id) + from cleveragents.actor.config import build_llm_kwargs_from_options + + llm_kwargs = build_llm_kwargs_from_options(actor_options, logger=self._logger) + llm = self._registry.create_llm( + provider_type=provider_type, model_id=model_id, **llm_kwargs + ) dod = definition_of_done or "Complete the plan objectives" prompt = ( @@ -402,6 +416,11 @@ class LLMExecuteActor: action = self._lifecycle.get_action(plan.action_name) actor_name = action.execution_actor or "openai/gpt-4" + # Resolve actor options BEFORE the name is resolved to + # provider/model (because the original namespace/name is + # needed for options lookup). + actor_options = self._lifecycle.resolve_actor_options(actor_name) or {} + # Pre-resolve namespace/name to provider/model via actor registry resolved = self._lifecycle.resolve_actor_provider_model(actor_name) if resolved: @@ -418,8 +437,13 @@ class LLMExecuteActor: ) try: + from cleveragents.actor.config import build_llm_kwargs_from_options + + llm_kwargs = build_llm_kwargs_from_options( + actor_options, logger=self._logger + ) llm = self._registry.create_llm( - provider_type=provider_type, model_id=model_id + provider_type=provider_type, model_id=model_id, **llm_kwargs ) except ValueError as exc: self._logger.warning( diff --git a/src/cleveragents/application/services/plan_lifecycle_service.py b/src/cleveragents/application/services/plan_lifecycle_service.py index 420ae4fb3..ff42bd165 100644 --- a/src/cleveragents/application/services/plan_lifecycle_service.py +++ b/src/cleveragents/application/services/plan_lifecycle_service.py @@ -760,6 +760,33 @@ class PlanLifecycleService: return None return f"{actor.provider}/{actor.model}" + def resolve_actor_options(self, actor_name: str) -> dict[str, Any] | None: + """Resolve a namespaced actor name to its opaque options dict. + + Returns the ``options`` key from the actor's ``config_blob`` when + available, or ``None`` when the actor cannot be found or has no + options configured. + """ + if not actor_name or actor_name.startswith("__"): + return None + if self.unit_of_work is None: + return None + try: + with self.unit_of_work.transaction() as ctx: + actor: Actor | None = ctx.actors.get_by_name(actor_name) + except Exception: + self._logger.warning( + "actor_options_resolution_failed", + actor_name=actor_name, + exc_info=True, + ) + return None + if actor is None: + return None + blob = actor.config_blob or {} + options = blob.get("options") if isinstance(blob, dict) else None + return dict(options) if isinstance(options, dict) else None + def _resolve_actor_registry_entry(self, actor_name: str) -> object | None: """Resolve a namespaced actor name to its stored configuration payload.""" if not actor_name or actor_name.startswith("__"): diff --git a/src/cleveragents/application/services/session_workflow.py b/src/cleveragents/application/services/session_workflow.py index 6791bfc15..358f4d454 100644 --- a/src/cleveragents/application/services/session_workflow.py +++ b/src/cleveragents/application/services/session_workflow.py @@ -130,6 +130,10 @@ class SessionWorkflow: tool_registry: Optional ``ToolRegistry`` (defaults to empty). llm_factory: Optional ``(actor_name: str) -> Any`` test injection point. Tests inject stub LLMs through this factory. + actor_resolver: Optional ``(actor_name: str) -> str | None`` for + namespace/name → provider/model resolution. + actor_options_resolver: Optional ``(actor_name: str) -> dict | None`` + for resolving actor-level options (openai_api_base, etc.). max_iterations: Max tool-call loop iterations (default 25). """ @@ -140,6 +144,7 @@ class SessionWorkflow: tool_registry: ToolRegistry | None = None, llm_factory: Callable[[str], Any] | None = None, actor_resolver: Callable[[str], str | None] | None = None, + actor_options_resolver: Callable[[str], dict[str, Any] | None] | None = None, max_iterations: int = 25, ) -> None: self._session_service = session_service @@ -147,6 +152,7 @@ class SessionWorkflow: self._tool_registry = tool_registry or ToolRegistry() self._llm_factory = llm_factory self._actor_resolver = actor_resolver + self._actor_options_resolver = actor_options_resolver self._max_iterations = max_iterations self._logger = logger.bind(component="session_workflow") # Populated by tell_stream() so callers can read real usage metrics @@ -401,9 +407,22 @@ class SessionWorkflow: effective_name = resolved if resolved is not None else actor_name provider_type, model_id = _parse_actor_name(effective_name) + actor_kwargs: dict[str, Any] = {} + if self._actor_options_resolver is not None: + try: + options = self._actor_options_resolver(actor_name) + if options: + from cleveragents.actor.config import build_llm_kwargs_from_options + + actor_kwargs = build_llm_kwargs_from_options( + options, logger=self._logger + ) + except Exception: + pass return self._provider_registry.create_llm( provider_type=provider_type, model_id=model_id, + **actor_kwargs, ) @staticmethod diff --git a/src/cleveragents/application/services/strategy_actor.py b/src/cleveragents/application/services/strategy_actor.py index 71284a6af..8bed908b6 100644 --- a/src/cleveragents/application/services/strategy_actor.py +++ b/src/cleveragents/application/services/strategy_actor.py @@ -33,6 +33,7 @@ from langchain_core.messages import HumanMessage, SystemMessage from pydantic import ValidationError as PydanticValidationError from ulid import ULID +from cleveragents.actor.config import build_llm_kwargs_from_options from cleveragents.application.services.context_tiers import ( ContextTierService, ) @@ -452,12 +453,20 @@ class StrategyActor: # Resolve actor name if lifecycle is available actor_name = _DEFAULT_ACTOR_NAME + actor_options: dict[str, Any] = {} if self._lifecycle is not None: try: plan = self._lifecycle.get_plan(plan_id) action = self._lifecycle.get_action(plan.action_name) actor_name = action.strategy_actor or _DEFAULT_ACTOR_NAME + # Resolve actor options BEFORE the name is resolved to + # provider/model (because the original namespace/name is + # needed for options lookup). + options_result = self._lifecycle.resolve_actor_options(actor_name) + if options_result: + actor_options = options_result + # Pre-resolve namespace/name to provider/model via actor registry resolved = self._lifecycle.resolve_actor_provider_model(actor_name) if resolved: @@ -479,7 +488,10 @@ class StrategyActor: model=model_id, ) - llm = self._registry.create_llm(provider_type=provider_type, model_id=model_id) + llm_kwargs = build_llm_kwargs_from_options(actor_options, logger=self._logger) + llm = self._registry.create_llm( + provider_type=provider_type, model_id=model_id, **llm_kwargs + ) # Gather ACMS context if pipeline or tier service available acms_context: str | None = None diff --git a/src/cleveragents/application/services/strategy_resolution.py b/src/cleveragents/application/services/strategy_resolution.py index 4790d6631..b3ce7eba6 100644 --- a/src/cleveragents/application/services/strategy_resolution.py +++ b/src/cleveragents/application/services/strategy_resolution.py @@ -42,6 +42,10 @@ class LifecycleService(Protocol): """Resolve a namespaced actor name to ``provider/model`` format.""" ... + def resolve_actor_options(self, actor_name: str) -> dict[str, Any] | None: + """Resolve a namespaced actor name to its opaque options dict.""" + ... + @runtime_checkable class AcmsPipeline(Protocol): @@ -198,6 +202,34 @@ def build_actor_resolver( return resolve +def build_actor_options_resolver( + actor_service: Any, +) -> Callable[[str], dict[str, Any] | None]: + """Build a namespace/name → options resolver closure. + + Returns a callable ``(actor_name: str) -> dict[str, Any] | None`` that + looks up an actor by its namespace/name reference and returns its + ``options`` dict from the stored config blob, or ``None`` when the + actor is unknown or has no options configured. + """ + + def resolve_options(actor_name: str) -> dict[str, Any] | None: + if not actor_name: + return None + parts = actor_name.split("/", 1) + if len(parts) == 2 and _is_known_provider(parts[0].strip()): + return None + try: + actor = actor_service.get_actor(actor_name) + except Exception: + return None + blob = getattr(actor, "config_blob", None) or {} + options = blob.get("options") if isinstance(blob, dict) else None + return dict(options) if isinstance(options, dict) else None + + return resolve_options + + def resolve_strategy_actor( provider_registry: ProviderRegistry | None = None, lifecycle_service: LifecycleService | None = None, diff --git a/src/cleveragents/cli/commands/session.py b/src/cleveragents/cli/commands/session.py index c4f80c198..b8e241f74 100644 --- a/src/cleveragents/cli/commands/session.py +++ b/src/cleveragents/cli/commands/session.py @@ -104,10 +104,12 @@ def _build_session_workflow() -> SessionWorkflow: service = _get_session_service() provider_registry = _get_provider_registry() actor_resolver = _build_actor_resolver() + actor_options_resolver = _build_actor_options_resolver() return SessionWorkflow( session_service=service, provider_registry=provider_registry, actor_resolver=actor_resolver, + actor_options_resolver=actor_options_resolver, ) @@ -160,6 +162,41 @@ def _null_actor_resolver(_actor_name: str) -> None: return None +def _null_actor_options_resolver(_actor_name: str) -> None: + """Null-object options resolver — always returns ``None``.""" + return None + + +def _build_actor_options_resolver(): + """Build a resolver callable for namespace/name -> actor options dict. + + Returns a callable ``(actor_name: str) -> dict | None`` that looks up + a namespace/name actor reference (e.g. ``"local/my-strategist"``) in + the actor registry and returns the actor's ``options`` dict from its + config blob, or ``None`` when the name is in provider/model format, + the actor is unknown, or the registry is unavailable. + """ + try: + from cleveragents.application.container import get_container + + container = get_container() + actor_service = container.actor_service() + if actor_service is None: + return _null_actor_options_resolver + + from cleveragents.application.services.strategy_resolution import ( + build_actor_options_resolver, + ) + + return build_actor_options_resolver(actor_service) + except Exception: + _log.warning( + "actor_options_resolver_unavailable", + exc_info=True, + ) + return _null_actor_options_resolver + + def _facade_dispatch(operation: str, params: dict[str, Any]) -> dict[str, Any]: """Route an operation through the A2A local facade. diff --git a/src/cleveragents/reactive/stream_router.py b/src/cleveragents/reactive/stream_router.py index 6a2082312..274bccc03 100644 --- a/src/cleveragents/reactive/stream_router.py +++ b/src/cleveragents/reactive/stream_router.py @@ -220,61 +220,19 @@ class SimpleLLMAgent: max_tokens = self.config.get("max_tokens") max_retries = self.config.get("max_retries") llm_kwargs: dict[str, Any] = {} + # Options are applied first so top-level keys take precedence over + # any duplicates in the options block. + options = dict(self.config.get("options") or {}) + from cleveragents.actor.config import build_llm_kwargs_from_options + + llm_kwargs.update(build_llm_kwargs_from_options(options, logger=logger_sr)) + if temperature is not None: llm_kwargs["temperature"] = temperature if max_tokens is not None: llm_kwargs["max_tokens"] = max_tokens if max_retries is not None: llm_kwargs["max_retries"] = max_retries - # M5: merge options block so custom LLM backend kwargs - # (e.g. openai_api_base, openai_api_key) are forwarded to the - # LLM constructor. Options are applied after the fixed keys so - # that explicit top-level keys (temperature, max_tokens, etc.) - # take precedence over duplicates in options. - # - # openai_api_key is handled specially: the registry uses a - # __api_key_sentinel mechanism to distinguish explicitly - # provided keys from environment-sourced ones. If the user - # supplies openai_api_key in options, we extract it and inject - # it via the sentinel so it overrides the registry's default. - options = dict(self.config.get("options") or {}) - if "openai_api_key" in options: - llm_kwargs["__api_key_sentinel"] = options.pop("openai_api_key") - # Ensure sensitive/reserved ChatOpenAI constructor params cannot - # be injected via a crafted actor YAML. - _ALLOWED_OPTIONS: frozenset[str] = frozenset( - { - "openai_api_base", - "temperature", - "max_tokens", - "timeout", - "top_p", - "frequency_penalty", - "presence_penalty", - } - ) - _RESERVED: frozenset[str] = frozenset({"provider_type", "model_id"}) - for key, value in options.items(): - if key in _RESERVED: - logger_sr.warning( - "Actor '%s' options block contains reserved key '%s' " - "that conflicts with positional arguments; ignoring.", - self.name, - key, - ) - continue - if key not in llm_kwargs: - if key in _ALLOWED_OPTIONS: - llm_kwargs[key] = value - else: - logger_sr.warning( - "Actor '%s' options block contains unrecognized " - "key '%s'; ignoring for security. Allowed keys: " - "%s.", - self.name, - key, - sorted(_ALLOWED_OPTIONS), - ) registry = get_provider_registry() self._llm = registry.create_llm( provider_type=provider, model_id=model, **llm_kwargs diff --git a/src/cleveragents/reactive/tool_caller.py b/src/cleveragents/reactive/tool_caller.py index bb6ace681..66d2ae8b4 100644 --- a/src/cleveragents/reactive/tool_caller.py +++ b/src/cleveragents/reactive/tool_caller.py @@ -131,6 +131,18 @@ class ToolCallingLLMCaller: max_retries = self._actor_config.get("max_retries") llm_kwargs: dict[str, Any] = {} + + # M7 (#11243): merge options block so custom LLM backend kwargs + # (e.g. openai_api_base, openai_api_key) are forwarded to the + # LLM constructor — mirrors the fix applied to SimpleLLMAgent in + # stream_router.py (PR #11225 / commit b3851693). + # Options are applied first so top-level keys take precedence over + # any duplicates in the options block. + options = dict(self._actor_config.get("options") or {}) + from cleveragents.actor.config import build_llm_kwargs_from_options + + llm_kwargs.update(build_llm_kwargs_from_options(options, logger=logger)) + if temperature is not None: llm_kwargs["temperature"] = temperature if max_tokens is not None: @@ -138,54 +150,6 @@ class ToolCallingLLMCaller: if max_retries is not None: llm_kwargs["max_retries"] = max_retries - # M7 (#11243): merge options block so custom LLM backend kwargs - # (e.g. openai_api_base, openai_api_key) are forwarded to the - # LLM constructor — mirrors the fix applied to SimpleLLMAgent in - # stream_router.py (PR #11225 / commit b3851693). - # - # Options are applied after the fixed keys so that explicit - # top-level keys (temperature, max_tokens, etc.) take precedence - # over any duplicate in the options block. - # - # openai_api_key is handled specially: the registry uses a - # __api_key_sentinel mechanism to distinguish explicitly provided - # keys from environment-sourced ones. If the user supplies - # openai_api_key in options, we extract it and inject it via the - # sentinel so it overrides the registry's default. - _ALLOWED_OPTIONS: frozenset[str] = frozenset( - { - "openai_api_base", - "temperature", - "max_tokens", - "timeout", - "top_p", - "frequency_penalty", - "presence_penalty", - } - ) - _RESERVED: frozenset[str] = frozenset({"provider_type", "model_id"}) - options = dict(self._actor_config.get("options") or {}) - if "openai_api_key" in options: - llm_kwargs["__api_key_sentinel"] = options.pop("openai_api_key") - for key, value in options.items(): - if key in _RESERVED: - logger.warning( - "Actor options block contains reserved key '%s' " - "that conflicts with positional arguments; ignoring.", - key, - ) - continue - if key not in llm_kwargs: - if key in _ALLOWED_OPTIONS: - llm_kwargs[key] = value - else: - logger.warning( - "Actor options block contains unrecognized key '%s'; " - "ignoring for security. Allowed keys: %s.", - key, - sorted(_ALLOWED_OPTIONS), - ) - registry = get_provider_registry() # Annotated as Any because create_llm() returns BaseLanguageModel but the # real runtime object is BaseChatModel which has bind_tools().