fix(runtime): add input validation, credential safety, and proper error propagation #40

2026-06-09T14:51:30Z

CoreRasurae commented

2026-06-09 14:51:30 +00:00

Summary

Fix critical bugs in the runtime Executor: missing input validation, incorrect error types, resource leaks, and missing credential propagation in graph execution.

Changes

Executor constructor: Add type validation for config_dict, credentials, limits, pricing parameters
_execute_llm: Raise ExecutionError (not ConfigurationError) for agent failures; add finally block with agent cleanup
_execute_llm: Guard credential access with None check before .get() calls
_execute_graph: Pass credentials to AgentFactory for proper agent creation
_execute_graph: Allow ConfigurationError/ExecutionError/AgentCreationError to propagate naturally
nodes.py: Remove hardcoded default workflow_controller routing in message router nodes

Commits

728630c fix(runtime): add input validation, credential safety, and proper error propagation

Closes #39

## Summary Fix critical bugs in the runtime Executor: missing input validation, incorrect error types, resource leaks, and missing credential propagation in graph execution. ## Changes - **Executor constructor**: Add type validation for `config_dict`, `credentials`, `limits`, `pricing` parameters - **`_execute_llm`**: Raise `ExecutionError` (not `ConfigurationError`) for agent failures; add `finally` block with agent cleanup - **`_execute_llm`**: Guard credential access with `None` check before `.get()` calls - **`_execute_graph`**: Pass `credentials` to `AgentFactory` for proper agent creation - **`_execute_graph`**: Allow `ConfigurationError`/`ExecutionError`/`AgentCreationError` to propagate naturally - **`nodes.py`**: Remove hardcoded default `workflow_controller` routing in message router nodes ## Commits - `728630c` fix(runtime): add input validation, credential safety, and proper error propagation Closes #39

CoreRasurae force-pushed fix/master-borked from 0e38052efa to 126fb22fab

2026-06-09 14:55:00 +00:00

Compare

CoreRasurae force-pushed fix/master-borked from 126fb22fab to 3f8c3dfad6

2026-06-09 14:57:07 +00:00

Compare

CoreRasurae force-pushed fix/master-borked from 3f8c3dfad6 to f8aebaf948

2026-06-09 15:03:33 +00:00

Compare

CoreRasurae force-pushed fix/master-borked from f8aebaf948 to 3fc0a3fa57

2026-06-09 15:06:42 +00:00

Compare

hurui200320 approved these changes 2026-06-09 15:14:55 +00:00

hurui200320 left a comment

PR Review — `fix(runtime): add input validation, credential safety, and proper error propagation`

Production Code Changes (2 files)

`src/cleveractors/langgraph/nodes.py` — 1 line changed

✅ Correct. Removing the hardcoded {"next_node": "workflow_controller"} fallback is the right call — routing to a node that may not exist in the graph was dangerous. Returning {} correctly hands routing back to the graph's own edge logic.

`src/cleveractors/runtime.py` — 52 additions, 13 deletions

1. Constructor validation ✅
Silently accepting garbage inputs was broken before. All four parameter guards are correct. Note that credentials is explicitly allowed to be None.

2. Credential safety in _execute_llm ✅
The old code called .get() unconditionally on self.credentials, which would crash with AttributeError when credentials=None. Now correctly guarded.

3. ExecutionError instead of ConfigurationError for agent failures ✅
Correct across _execute_llm, _execute_graph, and _execute_tool. Agent failures at runtime are execution errors, not configuration errors.

4. Agent cleanup finally blocks ✅
Resource leaks fixed in both _execute_llm and _execute_graph. The nested try/except in finally correctly ensures cleanup failures are silent and don't swallow the original exception.

5. Credentials passed to AgentFactory in _execute_graph ✅
This was the core bug breaking graph execution. Fixed.

6. Proper exception propagation ✅
Known typed exceptions (ConfigurationError, ExecutionError, AgentCreationError) are re-raised cleanly in all three execution paths; all others are wrapped in ExecutionError.

7. _estimate_tokens logging ✅
except Exception: pass → logger.debug(...). Better operability, no behaviour change.

Issues Found

🔶 Minor — `_build_factory_config()` unguarded `.items()` on `None` credentials

# _build_factory_config(), not changed in this PR
for provider, creds in self.credentials.items():

The constructor now explicitly allows credentials=None, but _build_factory_config() (unchanged) calls .items() unconditionally. If a caller passes credentials=None and the actor type resolves to "graph", this will raise AttributeError. This is pre-existing — the old code had the same issue. Non-blocking for this fix; recommend a follow-up: (self.credentials or {}).items().

🔵 Nit — Dead mock in `runtime_coverage_steps.py`

patch("cleveractors.runtime_tokens.estimate_graph_tokens", return_value=(100, 50)),

runtime.py no longer calls estimate_graph_tokens from runtime_tokens — it uses the private _estimate_tokens defined locally (which is correctly patched on the line above). This mock is now a no-op. Harmless but vestigial.

🔵 Nit — Inconsistent exception chaining

_execute_llm uses from None (chain suppressed); _execute_graph uses from exc (chain preserved). The suppression in _execute_llm appears intentional per the PR description. Minor — worth aligning in a follow-up.

Test Changes

Change	Assessment
`return_value="Mock graph response"` → `return_value=("Mock graph response", {})` in 3 step files	✅ Required — `_execute_graph` now unpacks `response, final_state = await graph.execute(...)`
Patch paths updated from `cleveractors.runtime.X` to actual module paths	✅ Required — `runtime.py` now uses lazy (local) imports; the old names no longer exist in the module namespace
New feature files + step definitions	✅ Additive — exercises new code paths introduced by this fix

Verdict

✅ Approved — safe to merge.

All five critical bugs from issue #39 are correctly addressed. The test fixes are consistent with the runtime behaviour changes. The two nits and the pre-existing _build_factory_config(None) edge case are non-blocking and can be addressed in a follow-up ticket.

## PR Review — `fix(runtime): add input validation, credential safety, and proper error propagation` ### Production Code Changes (2 files) #### `src/cleveractors/langgraph/nodes.py` — 1 line changed ✅ **Correct.** Removing the hardcoded `{"next_node": "workflow_controller"}` fallback is the right call — routing to a node that may not exist in the graph was dangerous. Returning `{}` correctly hands routing back to the graph's own edge logic. --- #### `src/cleveractors/runtime.py` — 52 additions, 13 deletions **1. Constructor validation** ✅ Silently accepting garbage inputs was broken before. All four parameter guards are correct. Note that `credentials` is explicitly allowed to be `None`. **2. Credential safety in `_execute_llm`** ✅ The old code called `.get()` unconditionally on `self.credentials`, which would crash with `AttributeError` when `credentials=None`. Now correctly guarded. **3. `ExecutionError` instead of `ConfigurationError` for agent failures** ✅ Correct across `_execute_llm`, `_execute_graph`, and `_execute_tool`. Agent failures at runtime are execution errors, not configuration errors. **4. Agent cleanup `finally` blocks** ✅ Resource leaks fixed in both `_execute_llm` and `_execute_graph`. The nested `try/except` in `finally` correctly ensures cleanup failures are silent and don't swallow the original exception. **5. Credentials passed to `AgentFactory` in `_execute_graph`** ✅ This was the core bug breaking graph execution. Fixed. **6. Proper exception propagation** ✅ Known typed exceptions (`ConfigurationError`, `ExecutionError`, `AgentCreationError`) are re-raised cleanly in all three execution paths; all others are wrapped in `ExecutionError`. **7. `_estimate_tokens` logging** ✅ `except Exception: pass` → `logger.debug(...)`. Better operability, no behaviour change. --- ### Issues Found #### 🔶 Minor — `_build_factory_config()` unguarded `.items()` on `None` credentials ```python # _build_factory_config(), not changed in this PR for provider, creds in self.credentials.items(): ``` The constructor now explicitly allows `credentials=None`, but `_build_factory_config()` (unchanged) calls `.items()` unconditionally. If a caller passes `credentials=None` and the actor type resolves to `"graph"`, this will raise `AttributeError`. **This is pre-existing** — the old code had the same issue. Non-blocking for this fix; recommend a follow-up: `(self.credentials or {}).items()`. #### 🔵 Nit — Dead mock in `runtime_coverage_steps.py` ```python patch("cleveractors.runtime_tokens.estimate_graph_tokens", return_value=(100, 50)), ``` `runtime.py` no longer calls `estimate_graph_tokens` from `runtime_tokens` — it uses the private `_estimate_tokens` defined locally (which is correctly patched on the line above). This mock is now a no-op. Harmless but vestigial. #### 🔵 Nit — Inconsistent exception chaining `_execute_llm` uses `from None` (chain suppressed); `_execute_graph` uses `from exc` (chain preserved). The suppression in `_execute_llm` appears intentional per the PR description. Minor — worth aligning in a follow-up. --- ### Test Changes | Change | Assessment | |---|---| | `return_value="Mock graph response"` → `return_value=("Mock graph response", {})` in 3 step files | ✅ Required — `_execute_graph` now unpacks `response, final_state = await graph.execute(...)` | | Patch paths updated from `cleveractors.runtime.X` to actual module paths | ✅ Required — `runtime.py` now uses lazy (local) imports; the old names no longer exist in the module namespace | | New feature files + step definitions | ✅ Additive — exercises new code paths introduced by this fix | --- ### Verdict **✅ Approved — safe to merge.** All five critical bugs from issue #39 are correctly addressed. The test fixes are consistent with the runtime behaviour changes. The two nits and the pre-existing `_build_factory_config(None)` edge case are non-blocking and can be addressed in a follow-up ticket.

CoreRasurae merged commit 3fc0a3fa57 into master

2026-06-09 15:20:55 +00:00

CoreRasurae deleted branch fix/master-borked

2026-06-09 15:20:55 +00:00

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: cleveragents/cleveractors-core#40

fix(runtime): add input validation, credential safety, and proper error propagation #40

Summary

Changes

Commits

PR Review — fix(runtime): add input validation, credential safety, and proper error propagation

Production Code Changes (2 files)

src/cleveractors/langgraph/nodes.py — 1 line changed

src/cleveractors/runtime.py — 52 additions, 13 deletions

Issues Found

🔶 Minor — _build_factory_config() unguarded .items() on None credentials

🔵 Nit — Dead mock in runtime_coverage_steps.py

🔵 Nit — Inconsistent exception chaining

Test Changes

Verdict

PR Review — `fix(runtime): add input validation, credential safety, and proper error propagation`

`src/cleveractors/langgraph/nodes.py` — 1 line changed

`src/cleveractors/runtime.py` — 52 additions, 13 deletions

🔶 Minor — `_build_factory_config()` unguarded `.items()` on `None` credentials

🔵 Nit — Dead mock in `runtime_coverage_steps.py`