feat(credentials): refactor LLMAgent/AgentFactory for per-request credential injection and extended provider routing #20
No reviewers
Labels
No labels
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Blocks
#12 feat(credentials): refactor LLMAgent/AgentFactory for per-request credential injection and extended provider routing
cleveragents/cleveractors-core
Reference
cleveragents/cleveractors-core!20
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/credential-injection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Implements Wave 3 of the cleveractors-core integration (ADR-2026, ADR-2028).
Closes #12
Summary
Refactors
LLMAgentandAgentFactoryto support per-request credential injection. The factory now extracts the provider-specific credential slice from the credentials dict (matching AC2) and passes it as a flatdict[str, str] | NonetoLLMAgent.LLMAgentstores the provider-specific credentials and passes them tobuild_chat_modelfor lazy client construction.Key Changes
AgentFactory
credentials: dict[str, dict[str, str]] | Noneparameter stored asself.credentials_create_agent_instancenow extractscredentials[provider]and passes the provider-specific slice toLLMAgentConfigurationErrorwhen credentials is not None but provider key is absent (AC4 compliance)ReactiveAgentFactorybackward-compatibility alias documented as submodule-only (not re-exported fromcleveractors.__init__)LLMAgent
credentials: dict[str, str] | None(provider-specific slice from factory)validate_credentials_structurecallchat_modelproperty (deferred to first access)_KNOWN_CLIENT_ATTRSpromoted toClassVarfor cleanup client discovery, with explicit provider coverage notes (ChatOpenAI, ChatAnthropic, ChatGoogleGenerativeAI)chat_modelgetter return type narrowed toBaseChatModelwith explicit None guardcredentialsproperty exposingself._credentialscleanup()acquires_chat_model_lockbefore reading_chat_modeland iterating clients (prevents double-close on concurrent calls — n4)temperature_overridetype validation inprocess_messageprocess_message()andcleanup()carry explicit concurrency-constraint docstring warnings (M5)Executor (runtime.py)
credentialsparameter changed todict[str, dict[str, str]] | Nonein bothExecutor.__init__andcreate_executor— standalone mode no longer requires passing{}(m4)_execute_graphusescopy.deepcopy(self.config)instead of shallow copy to prevent AgentFactory from mutating nested dicts (m5 / ADR-2026 AC8)from/toedge field fallbacks (aligns withvalidate_dict())_execute_llmSSRF Prevention (ADR-2028)
_validate_base_urlnow returns canonicalized URL with lowercased hostname and lowercased schemelocalhostcheck to prevent homograph bypasses likeℓocalhost(m7)%character in decoded hostname rejected to close IPv6 zone-ID bypass (n3)_check_known_provider_domainremovedllm_client.py (
_build_standalone)api_key: raisesConfigurationError("'api_key' must be a string, got <type>")immediately before the env-var fallback, preventing the misleading "Missing API key" error (m6)llm_providers.py (
validate_credentials_structure)LLMAgent.__init__as a caller (m2)llm.py module docstring
LLMAgentreceives a flat provider-specific credential slice, and that the provider-keyed lookup is performed byAgentFactory(m1)llm_imports.py
lazy_import_langchain→populate_langchain_globals(semantically accurate)CHANGELOG.md
ReactiveAgentFactoryalias entry moved from### Changedto### Added(m3)Tests
AgentFactory.create_agents_from_config()credential threadingQuality Gates
nox -e lintnox -e typechecknox -e unit_testsnox -e integration_testsPre-existing Issues (not addressed in this PR)
Implements Wave 3 of the cleveractors-core integration (ADR-2026, ADR-2028): **AgentFactory** - Added optional `credentials: dict[str, dict[str, str]] | None` parameter to `__init__`. Stored as `self.credentials`. - When creating `LLMAgent` instances, the full credentials dict is forwarded via a new `credentials` constructor kwarg, threading per-request API keys without touching the stored actor `config_dict`. **LLMAgent** - Added optional `credentials: dict[str, dict[str, str]] | None` parameter to `__init__`. Stored as `self._credentials`; config_dict is never modified. - LangChain client construction deferred to first access (lazy init) via a `chat_model` property backed by `_ensure_chat_model()`. The property setter allows test-injection of mock models without going through the lazy-init path. - `_create_chat_model()` now dispatches to two sub-paths: - **Credential-injection path** (when `credentials` is not None): looks up `credentials[provider]`. Missing entry raises `ConfigurationError("missing credentials for provider: <name>")`. Non-native providers (anything outside {openai, anthropic, google}) route to `ChatOpenAI(base_url=..., api_key=...)`. The `openai_compatible` provider value is treated identically, keyed as `credentials["openai_compatible"]`. - **Standalone/CLI path** (when `credentials` is None): reads `config["api_key"]` then falls back to env vars (OPENAI_API_KEY, etc.). Non-native providers raise `ConfigurationError("Unsupported provider: ...")` in this mode. **Tests** - New `features/credential_injection.feature` + step module: 23 BDD scenarios covering AgentFactory credentials param, lazy init behaviour, credential- injection path for native and non-native providers, missing-credentials errors, env-var fallback, config immutability, factory-to-agent threading, and process_message end-to-end with injected credentials. - Updated `features/steps/llm_agent_steps.py`: - `step_try_create_llm_agent`: accesses `chat_model` after construction to trigger lazy-init errors (deferred from __init__ intentionally). - `step_create_google_agent`: patches `ChatGoogleGenerativeAI` and calls `_ensure_chat_model()` within the patch scope instead of patching during construction (no-op with lazy init). - `step_initialized_google_agent`: injects mock directly via `chat_model` setter. - Updated `features/steps/llm_missing_coverage_steps.py`: - `step_ml_try_construct_no_key`: triggers lazy init after construction. - `step_ml_config_no_key`: strips surrounding quotes from provider name captured by Behave (pre-existing test data bug now visible after wrapping removal). - Added assertion message to `step_ml_error_mentions_key` for better diagnostics. Quality gates (all passing): nox -e lint Pass nox -e typecheck Pass (0 errors, 1 import warning for optional pkg) nox -e unit_tests Pass (1727 scenarios) nox -e integration_tests Pass (54 tests) nox -e coverage_report Pass 96.9% (threshold 96.5%) ISSUES CLOSED: #1291378529d7fd361b38fafd361b38fatoffa7faff2bffa7faff2b390ac75e3b390ac75e3bd0727a4d90d0727a4d90e76345219ae76345219a64988fccfe64988fccfe0201f8bfe70201f8bfe7ab0ea12fceab0ea12fce24ba92c37124ba92c3712fd363fbe32fd363fbe3438bc21f00438bc21f00de1f7b09ccde1f7b09cc17c485c21817c485c218da8448820dda8448820df7aa076f9df7aa076f9d0dd490cd3b0dd490cd3ba48d137c63a48d137c63c7a733dcf4c7a733dcf4ed9a479f35ed9a479f352050363b7e2050363b7eb8c3585d49b8c3585d496dc684497c6dc684497c858ffccebb858ffccebb4801892e254801892e2561b90c6b1b61b90c6b1bca9bde13a6ca9bde13a6b1f34f1714b1f34f17148f6df3687a8f6df3687a336297e6ef336297e6ef49938a530649938a5306f281fa3b24Sanity-Check Review (src/ only)
Ticket Requirements (Issue #12) — All 9 ACs covered ✅
AgentFactoryacceptscredentialsparamfactory.py.__init__credentials[provider]slice forLLMAgentfactory.py._create_agent_instanceLLMAgentlazy-inits chat modelllm.py._ensure_chat_model+_chat_model_lockConfigurationErrorwhen provider key missing from credentialsfactory.pymissing-key guardcredentials=Nonellm_client._build_standaloneChatOpenAI(base_url=...)llm_client._build_from_credentialsopenai_compatibletreated identically_NATIVE_PROVIDERS, falls through to non-native pathconfig_dictnever mutateddeepcopyin_execute_graph;setdefaulton a shallow copy in_execute_llmWill merging break anything?
Short answer: No, not for well-formed inputs.
There are three behavioural changes — all intentional improvements, not regressions:
a. Edge field
"from"/"to"fallback removed (runtime.py)Old code silently tolerated legacy
from/tokeys. New code does a hardedge_def["source"]lookup. Only affects callers constructingroutedicts manually while bypassing the schema validator; any config that passedvalidate_dict()already usessource/targetand is unaffected.b. Unknown composite-agent references now raise
AgentCreationError(factory.py)Previously a missing agent in a composite was silently skipped, producing silently broken behaviour at runtime. Now it fails immediately with a clear error. Correctness fix.
c.
create_executorcredentials type tightenedChanged from required
dict[str, Any]to optionaldict[str, dict[str, str]] | None(defaultNone).validate_credentials_structurerejects non-string credential values. Existing well-formed callers are unaffected; callers passing{"openai": {"api_key": 123}}would now get aConfigurationErrorinstead of a misleading downstream failure.Key implementation correctness notes
api_keyresolution:api_keynested underconfig_dict["config"]["api_key"]reaches_build_standalone; a top-levelconfig_dict["api_key"]does not — but this matches the old code's behaviour exactly, so no regression.BaseChatModelannotation: Only imported underTYPE_CHECKING;from __future__ import annotationsmakes all annotations strings at runtime — noNameError. ✅populate_langchain_globalslocking: threading lock (not asyncio), called from sync__init__, guarded section is pure dict assignment. No deadlock risk with_chat_model_lock. ✅cleanup()lock ordering: acquires lock, collects clients, sets_chat_model = None, then releases lock before awaitingclose()— correct, prevents holding a thread lock across async I/O. ✅finally: readsself._chat_modeldirectly (bypasses the property getter) and guards withis not Noneto handle the case wherecleanup()ran concurrently. ✅Verdict: Safe to merge ✅
All 9 ACs from issue #12 are delivered. The three behavioural changes are all improvements (fail-fast instead of silent-wrong). No production regression risk for callers using the public API with valid inputs.
The 3 previously failing BDD scenarios (
runtime_coverage.feature:29,:51,:72) have been fixed in the same commit — wrongunittest.mock.patchtargets after imports were moved to module level, plus a stale legacy edge-key in the test fixture.