Skip to content

Commit be37e85

Browse files
aditik0303claude
andcommitted
fix(agent-framework): address remaining review minors (page 5)
Follow-up to the #361 review pass — the earlier commit covered streaming (stream_result_hook), try/finally, trace_id, counters, and the _stringify cap; these are the rest: - middleware.py:113 (walk gap — page-2 theme + minor 8) — _iter_agents now RECURSES through nested WorkflowAgents (workflow-of-workflows). Before it stopped one level down, so a nested workflow's inner agents ran ungoverned. Converted to an explicit stack walk with a _MAX_GRAPH_NODES cap that logs when it trips (was a silent inline `len(seen) < 1000`). - middleware.py:326 (_coerce_args) — log a warning when model_dump() fails instead of silently dropping the tool args from governance visibility. - middleware.py:135 (nit) — inverted the empty-targets check to a guard clause. Tests: nested-workflow recursion + cycle-safety (self-referential executor); streaming + AFTER_MODEL-on-error were added in the earlier pass. 141 pass. Acknowledged, unchanged: latest-message-only scan (same documented tradeoff as #899); cached-agent re-attach does not apply — this factory does not cache agents (fresh per runtime) and trace_id is no longer held. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 387e982 commit be37e85

2 files changed

Lines changed: 67 additions & 24 deletions

File tree

packages/uipath-agent-framework/src/uipath_agent_framework/governance/middleware.py

Lines changed: 48 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@
6464
# prompt — see :meth:`GovernanceCallbacks._latest_message_text`.
6565
_BEFORE_MODEL_TEXT_CAP = 64000
6666

67+
# Hard cap on how many nodes the workflow walk visits, guarding against cyclic
68+
# or pathologically deep (nested) workflows. Hitting it is logged, not silent.
69+
_MAX_GRAPH_NODES = 1000
70+
6771

6872
def install_governance(
6973
agent: Any,
@@ -87,6 +91,13 @@ def install_governance(
8791
evaluator=evaluator, agent_name=agent_name, session_id=session_id
8892
)
8993
targets = _iter_agents(agent)
94+
if not targets:
95+
logger.warning(
96+
"install_governance found no agent in %s — hooks will not fire",
97+
type(agent).__name__,
98+
)
99+
return agent
100+
90101
installed = 0
91102
for node in targets:
92103
existing = list(getattr(node, "middleware", None) or [])
@@ -99,42 +110,48 @@ def install_governance(
99110
*existing,
100111
]
101112
installed += 1
102-
if not targets:
103-
logger.warning(
104-
"install_governance found no agent in %s — hooks will not fire",
105-
type(agent).__name__,
106-
)
107-
else:
108-
logger.debug("Installed governance middleware on %d agent(s)", installed)
113+
logger.debug("Installed governance middleware on %d agent(s)", installed)
109114
return agent
110115

111116

112117
def _iter_agents(root: Any) -> List[Any]:
113118
"""Return every agent node carrying a ``middleware`` slot.
114119
115120
A plain ``Agent`` is itself the target. A ``WorkflowAgent`` exposes its
116-
inner agents through ``workflow.executors[*]._agent`` (the same traversal
117-
the breakpoint middleware uses), so a multi-agent app is governed end to
118-
end. Cycles / pathological size are bounded by an id-visited set and a cap.
121+
inner agents through ``workflow.executors[*]._agent``. Those inner agents
122+
can themselves be ``WorkflowAgent``s (workflow-of-workflows), so the walk
123+
**recurses** through nested workflows rather than stopping one level down —
124+
otherwise a nested workflow's agents would run ungoverned. Cycles and
125+
pathological depth are bounded by an id-visited set and a hard cap
126+
(``_MAX_GRAPH_NODES``), which logs rather than silently truncating.
119127
"""
120128
found: List[Any] = []
121129
seen: set[int] = set()
122-
123-
def _add(node: Any) -> None:
130+
stack: List[Any] = [root]
131+
capped = False
132+
while stack:
133+
if len(seen) >= _MAX_GRAPH_NODES:
134+
capped = True
135+
break
136+
node = stack.pop()
124137
if node is None or id(node) in seen:
125-
return
138+
continue
126139
seen.add(id(node))
127140
if hasattr(node, "middleware"):
128141
found.append(node)
129-
130-
_add(root)
131-
workflow = getattr(root, "workflow", None)
132-
executors = getattr(workflow, "executors", None)
133-
if isinstance(executors, Mapping):
134-
for executor in list(executors.values()):
135-
inner = getattr(executor, "_agent", None)
136-
if inner is not None and len(seen) < 1000:
137-
_add(inner)
142+
workflow = getattr(node, "workflow", None)
143+
executors = getattr(workflow, "executors", None)
144+
if isinstance(executors, Mapping):
145+
for executor in executors.values():
146+
inner = getattr(executor, "_agent", None)
147+
if inner is not None:
148+
stack.append(inner)
149+
if capped:
150+
logger.warning(
151+
"install_governance stopped walking the agent graph at the %d-node "
152+
"cap; agents beyond it will not be governed",
153+
_MAX_GRAPH_NODES,
154+
)
138155
return found
139156

140157

@@ -359,8 +376,15 @@ def _coerce_args(arguments: Any) -> Dict[str, Any]:
359376
dumped = model_dump()
360377
if isinstance(dumped, dict):
361378
return dumped
362-
except Exception: # noqa: BLE001 - fall through to empty
363-
pass
379+
except Exception as e: # noqa: BLE001
380+
# Don't silently drop the args from governance visibility — surface
381+
# that they couldn't be coerced.
382+
logger.warning(
383+
"governance: could not coerce %s tool args to a dict (%s); "
384+
"TOOL_CALL will see empty args",
385+
type(arguments).__name__,
386+
e,
387+
)
364388
return {}
365389

366390

packages/uipath-agent-framework/tests/governance/test_middleware.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,25 @@ def test_install_governance_installs_on_workflow_inner_agents():
125125
assert any(isinstance(m, GovernanceChatMiddleware) for m in node.middleware)
126126

127127

128+
def test_install_governance_recurses_into_nested_workflow():
129+
"""A WorkflowAgent inside a WorkflowAgent (workflow-of-workflows): the deep
130+
leaf agent must still be governed, not left one level below the walk."""
131+
leaf = FakeAgent("leaf")
132+
inner = FakeWorkflowAgent([leaf])
133+
root = FakeWorkflowAgent([inner])
134+
install_governance(root, FakeEvaluator(), agent_name="x", session_id="s")
135+
assert any(isinstance(m, GovernanceChatMiddleware) for m in leaf.middleware)
136+
137+
138+
def test_install_governance_is_cycle_safe():
139+
"""A workflow whose executor points back at itself must not loop forever."""
140+
w = FakeWorkflowAgent([])
141+
w.workflow.executors = {"self": SimpleNamespace(_agent=w)}
142+
# completes (id-visited set breaks the cycle) and governs w exactly once
143+
install_governance(w, FakeEvaluator(), agent_name="x", session_id="s")
144+
assert sum(isinstance(m, GovernanceChatMiddleware) for m in w.middleware) == 1
145+
146+
128147
def test_install_governance_is_idempotent():
129148
agent = FakeAgent()
130149
ev = FakeEvaluator()

0 commit comments

Comments
 (0)