Skip to content

Commit 277b34f

Browse files
committed
fix(aws-strands): refuse plugins at wrap time below the SDK floor
The plugins kwarg added in the previous commit reached a constructor that has no such parameter on strands-agents below 1.28.0, the release that added plugins to Agent. This package declares a 1.15.0 floor and runs a CI lane against it. There, Strands raised a bare TypeError from inside per-thread construction, and because that happens inside the run generator it escaped to the caller rather than becoming a run error: the first request died with an SDK traceback pointing at neither the argument nor the version that could not take it. Probe the capability off the Agent constructor signature and refuse at wrap time instead. A static misconfiguration is knowable the moment the wrapper is built, so it is answered there, with a message naming the installed version and what to do about it. Not raised for a multi-agent orchestrator, which builds no per-thread agent and so ignores plugins on every release; reporting a version problem there would describe something the newer releases do not do either. Tests previously gated the whole plugin block on the SDK having plugins, which left twelve behaviours unasserted at the declared floor. Only one of them needs the real plugin system: the one proving init_agent runs once per thread against a real Agent. The rest exercise the adapter's own reading, reporting and forwarding, so they now use a synthetic plugin registry and plain sentinels, the way the resolver tests in that file already do, and run on every supported release. The three that drive the kwarg against a stub core declare the capability explicitly rather than skipping, which is what the stub already assumes. Floor skips drop from twelve to one. Adds direct coverage for the SDK-plugin name filter and for the new refusal, and a README section documenting the per-thread rebuild, the hooks and plugins routes, and the version boundary.
1 parent 7119f8e commit 277b34f

4 files changed

Lines changed: 236 additions & 85 deletions

File tree

integrations/aws-strands/python/README.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,41 @@ The integration has three main layers:
7575

7676
See [ARCHITECTURE.md](../ARCHITECTURE.md) for diagrams and a deeper dive.
7777

78+
## Per-thread agents: hooks and plugins
79+
80+
The wrapper does not run the agent you hand it. That one is a template: the
81+
adapter reads its constructor settings back off the instance and builds a fresh
82+
`strands.Agent` per `thread_id`, so one conversation cannot see another's
83+
history. Most settings survive that rebuild automatically.
84+
85+
Two do not, because Strands consumes them during construction rather than
86+
keeping the list you passed. Hooks become a `HookRegistry`, and plugins are run
87+
against the agent that received them and recorded in a registry bound to it.
88+
Neither can be read back or handed to a second agent, so a template is the one
89+
place they will not work. Pass them to the wrapper instead and every per-thread
90+
agent gets its own:
91+
92+
```python
93+
agui_agent = StrandsAgent(
94+
agent=strands_agent,
95+
name="my_agent",
96+
hooks=[MyHookProvider()],
97+
plugins=[AgentSkills(skills="./skills/")],
98+
)
99+
```
100+
101+
Set either on the template and the adapter logs a warning naming the setting
102+
the first time a thread is built, rather than dropping it in silence. For a
103+
value that has to differ per thread, build it in
104+
`StrandsAgentConfig.thread_agent_kwargs`, which runs per request and wins over
105+
both routes above.
106+
107+
| Scenario | Support boundary |
108+
| ------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
109+
| `hooks=[...]` | Supported on every release this package supports. |
110+
| `plugins=[...]` | Requires `strands-agents >= 1.28.0`, the release that added `plugins` to `Agent`. On an older release the wrapper raises `TypeError` when it is constructed, not on the first request. |
111+
| `hooks` / `plugins` with a multi-agent orchestrator | Ignored. An orchestrator is invoked directly, so there is no per-thread agent to attach them to. |
112+
78113
## Key Files
79114

80115
| File | Description |

integrations/aws-strands/python/src/ag_ui_strands/agent.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,17 @@ def _read(prop: property) -> Any:
176176
return _MISSING
177177

178178

179+
# Whether the installed Strands takes ``plugins`` on its Agent constructor.
180+
# The plugin system arrived after this package's declared strands-agents floor,
181+
# so the adapter's own ``plugins=`` kwarg can be handed a release with nowhere
182+
# to put it. Probed off the signature rather than compared against a version,
183+
# for the same reason the forwarding probe is: what matters is the parameter
184+
# being there, not which release put it there.
185+
_STRANDS_ACCEPTS_PLUGINS = (
186+
"plugins" in inspect.signature(StrandsAgentCore.__init__).parameters
187+
)
188+
189+
179190
# Strands namespaces the plugins it registers on every Agent itself, and
180191
# registers them whether or not the caller passed any. Anything under this
181192
# prefix is therefore the SDK's, not a setting to report as dropped.
@@ -3170,6 +3181,29 @@ def __init__(
31703181
# nothing. Taking them from the caller instead lets every per-thread
31713182
# agent build its own.
31723183
self._plugins = list(plugins) if plugins else []
3184+
# Refused at wrap time rather than on the first request. Without this
3185+
# the kwarg reaches a constructor with no parameter for it and Strands
3186+
# raises a bare TypeError from inside per-thread construction, which
3187+
# escapes the run generator: the caller sees a traceback pointing at
3188+
# the SDK rather than at the argument they passed, and only once a
3189+
# request arrives. This is a static misconfiguration, knowable the
3190+
# moment the wrapper is built, so it is answered there.
3191+
# Not raised for an orchestrator, which never builds a per-thread agent
3192+
# and so ignores plugins on every release. Refusing only the old ones
3193+
# there would report a version problem for something the new ones do
3194+
# not do either.
3195+
if (
3196+
self._plugins
3197+
and self._orchestrator is None
3198+
and not _STRANDS_ACCEPTS_PLUGINS
3199+
):
3200+
raise TypeError(
3201+
"plugins= was supplied, but the installed strands-agents "
3202+
f"({distribution_version('strands-agents')}) has no `plugins` "
3203+
"parameter on Agent, so they cannot be forwarded to per-thread "
3204+
"agents. Upgrade strands-agents to a release that supports "
3205+
"plugins, or drop the argument."
3206+
)
31733207

31743208
self.name = name
31753209
self.description = description

integrations/aws-strands/python/tests/test_template_agent_propagation.py

Lines changed: 133 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import types
2424
import typing
2525
import warnings
26+
import weakref
2627
from unittest.mock import MagicMock, patch
2728

2829
import pytest
@@ -32,6 +33,8 @@
3233
from ag_ui_strands.agent import (
3334
StrandsAgent,
3435
_AGUI_EXPLICIT_PARAMS,
36+
_STRANDS_ACCEPTS_PLUGINS,
37+
_template_plugin_names,
3538
_extract_agent_kwargs,
3639
_forwardable_parameters,
3740
_references_agent,
@@ -42,29 +45,23 @@
4245
)
4346

4447

45-
# The plugin system arrived after the declared strands-agents floor, and the
46-
# floor is one of the lanes this suite runs in. Probed as a capability rather
47-
# than a version so the gate tracks the feature itself.
48-
#
49-
# This is not the skip the module docstring rules out. That one is about a
50-
# param the installed SDK has and this suite finds awkward; here the SDK has no
51-
# plugins at all, so there is no setting to carry and nothing to be silent
52-
# about.
48+
# Only the one test below that drives a real ``strands.Agent`` with a real
49+
# plugin needs the SDK's plugin system. Everything else here exercises the
50+
# adapter's own reading, reporting and forwarding, which are the adapter's
51+
# code on every release, so those tests run at the declared strands-agents
52+
# floor too. Keeping the gate that narrow is deliberate: a skip at the floor
53+
# is a version of this package nobody checked.
5354
try:
5455
from strands.plugins import Plugin as _StrandsPlugin
5556
except ImportError: # pragma: no cover - depends on the installed SDK
5657
_StrandsPlugin = None
5758

58-
_NO_PLUGIN_SUPPORT = (
59-
_StrandsPlugin is None
60-
or "plugins" not in inspect.signature(Agent.__init__).parameters
61-
)
62-
_needs_plugins = pytest.mark.skipif(
63-
_NO_PLUGIN_SUPPORT,
64-
reason="this strands-agents release has no plugin system to carry",
59+
_needs_sdk_plugins = pytest.mark.skipif(
60+
_StrandsPlugin is None or not _STRANDS_ACCEPTS_PLUGINS,
61+
reason="this strands-agents release has no plugin system to drive",
6562
)
66-
# Subclassable stand-in so the plugin classes below still import on a release
67-
# without them. Every test that touches one is skipped there.
63+
# Subclassable stand-in so the plugin class below still imports on a release
64+
# without them. The single test that touches it is skipped there.
6865
_PluginBase = _StrandsPlugin if _StrandsPlugin is not None else object
6966

7067

@@ -978,26 +975,59 @@ async def test_no_warning_for_a_param_the_hook_supplies(caplog):
978975
# second agent, so a plugin set on the template never runs against the agents
979976
# that serve requests. The adapter answers that with a dedicated kwarg, and
980977
# tells anyone who used the template instead.
978+
#
979+
# Most of what follows is the adapter reading, reporting and forwarding, none
980+
# of which needs a real plugin. Those tests use a synthetic registry and plain
981+
# sentinels, the way the resolver tests above already do, so they run on every
982+
# supported release rather than only the ones new enough to have plugins.
981983

982984

983-
class _CountingPlugin(_PluginBase):
984-
"""Records how many agents it was initialized against."""
985+
class _FakePluginRegistry:
986+
"""A plugin registry in the shape the adapter reads.
985987
986-
name = "counting-plugin"
988+
Bound to its agent by weak reference and keyed by plugin name, which is
989+
what a real ``_PluginRegistry`` is. Built here rather than by constructing
990+
a real Agent with plugins so these tests still run at the declared
991+
strands-agents floor, which has no plugin system at all.
992+
"""
987993

988-
def __init__(self):
989-
super().__init__()
990-
self.agents: list = []
994+
def __init__(self, owner, plugins: dict):
995+
self._agent_ref = weakref.ref(owner)
996+
self._plugins = dict(plugins)
991997

992-
def init_agent(self, agent):
993-
self.agents.append(agent)
998+
999+
class _NamedPlugin:
1000+
"""The only thing the adapter reads off a plugin is its name."""
1001+
1002+
def __init__(self, name: str):
1003+
self.name = name
1004+
1005+
1006+
def _template_with_plugins(*names: str):
1007+
"""A real Agent carrying a registry of caller plugins under those names."""
1008+
agent = Agent(model=_mock_model())
1009+
agent._plugin_registry = _FakePluginRegistry(
1010+
agent, {name: _NamedPlugin(name) for name in names}
1011+
)
1012+
return agent
9941013

9951014

9961015
def _plugin_warnings(messages: list[str]) -> list[str]:
9971016
return [m for m in messages if "plugins" in m]
9981017

9991018

1000-
@_needs_plugins
1019+
def _as_if_sdk_took_plugins():
1020+
"""Assert the forwarding on a release whose real Agent would refuse it.
1021+
1022+
The per-thread core is a stub in these tests, and a stub takes any kwarg.
1023+
What stands between them and running at the declared floor is the wrap-time
1024+
capability check, so declaring the capability is the whole adaptation. It
1025+
says out loud what the stub already assumes, which is better than skipping
1026+
and leaving the adapter's own forwarding logic unasserted on that release.
1027+
"""
1028+
return patch("ag_ui_strands.agent._STRANDS_ACCEPTS_PLUGINS", True)
1029+
1030+
10011031
@pytest.mark.asyncio
10021032
async def test_template_plugins_are_named_when_a_thread_is_built(caplog):
10031033
"""The silence this closes: plugins on the template, and no plugins anywhere.
@@ -1007,8 +1037,7 @@ async def test_template_plugins_are_named_when_a_thread_is_built(caplog):
10071037
the worst of the two failure modes: nothing to notice and nothing to
10081038
search for.
10091039
"""
1010-
template = Agent(model=_mock_model(), plugins=[_CountingPlugin()])
1011-
ag = StrandsAgent(template, name="test")
1040+
ag = StrandsAgent(_template_with_plugins("mine"), name="test")
10121041

10131042
with caplog.at_level(logging.WARNING, logger="ag_ui_strands.agent"):
10141043
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
@@ -1020,7 +1049,6 @@ async def test_template_plugins_are_named_when_a_thread_is_built(caplog):
10201049
)
10211050

10221051

1023-
@_needs_plugins
10241052
@pytest.mark.asyncio
10251053
async def test_an_agent_with_no_caller_plugins_says_nothing(caplog):
10261054
"""Strands registers plugins of its own on every Agent.
@@ -1042,33 +1070,49 @@ async def test_an_agent_with_no_caller_plugins_says_nothing(caplog):
10421070
)
10431071

10441072

1045-
@_needs_plugins
1073+
def test_the_sdks_own_plugins_are_not_read_as_the_callers():
1074+
"""The filter that keeps the warning off every caller, asserted directly.
1075+
1076+
Driven against a synthetic registry so it states the rule rather than
1077+
whichever built-ins the installed release happens to register.
1078+
"""
1079+
agent = Agent(model=_mock_model())
1080+
agent._plugin_registry = _FakePluginRegistry(
1081+
agent,
1082+
{"strands:model": _NamedPlugin("strands:model"), "mine": _NamedPlugin("mine")},
1083+
)
1084+
1085+
assert _template_plugin_names(agent) == ["mine"], (
1086+
"expected only the caller's plugin to be reported as uncarried"
1087+
)
1088+
1089+
10461090
@pytest.mark.asyncio
10471091
async def test_no_plugins_warning_when_the_explicit_kwarg_supplies_them(caplog):
10481092
"""Acting on the warning has to make it stop.
10491093
10501094
The kwarg is what the message asks for, so a caller who has already used
10511095
it must not keep hearing about the template.
10521096
"""
1053-
template = Agent(model=_mock_model(), plugins=[_CountingPlugin()])
1054-
ag = StrandsAgent(template, name="test", plugins=[_CountingPlugin()])
1097+
with _as_if_sdk_took_plugins():
1098+
ag = StrandsAgent(
1099+
_template_with_plugins("on-template"), name="test", plugins=[object()]
1100+
)
10551101

1056-
with caplog.at_level(logging.WARNING, logger="ag_ui_strands.agent"):
1057-
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
1058-
await _trigger_thread_creation(ag, "t1")
1102+
with caplog.at_level(logging.WARNING, logger="ag_ui_strands.agent"):
1103+
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
1104+
await _trigger_thread_creation(ag, "t1")
10591105

10601106
assert not _plugin_warnings(caplog.messages), (
10611107
f"warned about plugins the caller had already supplied; "
10621108
f"got {caplog.messages}"
10631109
)
10641110

10651111

1066-
@_needs_plugins
10671112
@pytest.mark.asyncio
10681113
async def test_plugins_are_only_warned_about_once(caplog):
10691114
"""One thread's message must not become every thread's message."""
1070-
template = Agent(model=_mock_model(), plugins=[_CountingPlugin()])
1071-
ag = StrandsAgent(template, name="test")
1115+
ag = StrandsAgent(_template_with_plugins("mine"), name="test")
10721116

10731117
with caplog.at_level(logging.WARNING, logger="ag_ui_strands.agent"):
10741118
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
@@ -1084,12 +1128,10 @@ async def test_plugins_are_only_warned_about_once(caplog):
10841128
)
10851129

10861130

1087-
@_needs_plugins
10881131
@pytest.mark.asyncio
10891132
async def test_the_plugins_warning_points_at_the_kwarg_that_fixes_it():
10901133
"""A message naming the problem and not the route is half a message."""
1091-
template = Agent(model=_mock_model(), plugins=[_CountingPlugin()])
1092-
ag = StrandsAgent(template, name="test")
1134+
ag = StrandsAgent(_template_with_plugins("mine"), name="test")
10931135

10941136
with patch.object(logging.getLogger("ag_ui_strands.agent"), "warning") as warn:
10951137
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
@@ -1103,16 +1145,20 @@ async def test_the_plugins_warning_points_at_the_kwarg_that_fixes_it():
11031145
)
11041146

11051147

1106-
@_needs_plugins
11071148
@pytest.mark.asyncio
11081149
async def test_plugins_kwarg_reaches_the_per_thread_agent():
1109-
"""The forwarding half: what the caller passes is what the thread gets."""
1110-
plugin = _CountingPlugin()
1111-
template = Agent(model=_mock_model())
1112-
ag = StrandsAgent(template, name="test", plugins=[plugin])
1150+
"""The forwarding half: what the caller passes is what the thread gets.
11131151
1114-
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
1115-
instance = await _trigger_thread_creation(ag, "t1")
1152+
A sentinel rather than a real plugin, because the adapter's job here is to
1153+
hand the list on unexamined. What a real plugin then does with a real
1154+
Agent is asserted separately below.
1155+
"""
1156+
plugin = object()
1157+
with _as_if_sdk_took_plugins():
1158+
ag = StrandsAgent(Agent(model=_mock_model()), name="test", plugins=[plugin])
1159+
1160+
with patch("ag_ui_strands.agent.StrandsAgentCore", _CapturingCore):
1161+
instance = await _trigger_thread_creation(ag, "t1")
11161162

11171163
assert "plugins" in instance.init_kwargs, (
11181164
f"plugins kwarg never reached the per-thread constructor; "
@@ -1124,7 +1170,6 @@ async def test_plugins_kwarg_reaches_the_per_thread_agent():
11241170
)
11251171

11261172

1127-
@_needs_plugins
11281173
@pytest.mark.asyncio
11291174
@pytest.mark.parametrize(
11301175
"plugins_value",
@@ -1149,13 +1194,49 @@ async def test_a_falsy_plugins_value_omits_the_kwarg(plugins_value):
11491194
)
11501195

11511196

1152-
@_needs_plugins
1197+
def test_plugins_on_a_release_without_them_is_refused_at_wrap_time():
1198+
"""A release below the plugin system gets an answer, not a traceback.
1199+
1200+
The declared strands-agents floor has no ``plugins`` parameter at all.
1201+
Left alone, the kwarg reached that constructor and Strands raised a bare
1202+
TypeError from inside per-thread construction, which escapes the run
1203+
generator: the caller saw an SDK traceback on their first request rather
1204+
than a sentence about the argument they passed. Refusing while the wrapper
1205+
is being built says it once, at the point the mistake was made.
1206+
"""
1207+
template = Agent(model=_mock_model())
1208+
1209+
with patch("ag_ui_strands.agent._STRANDS_ACCEPTS_PLUGINS", False):
1210+
with pytest.raises(TypeError, match="plugins"):
1211+
StrandsAgent(template, name="test", plugins=[object()])
1212+
1213+
# Not asking for the feature is not a misconfiguration, so the same
1214+
# release must still build a wrapper that never mentions plugins.
1215+
StrandsAgent(template, name="test")
1216+
StrandsAgent(template, name="test", plugins=[])
1217+
1218+
1219+
class _CountingPlugin(_PluginBase):
1220+
"""Records which agents it was initialized against."""
1221+
1222+
name = "counting-plugin"
1223+
1224+
def __init__(self):
1225+
super().__init__()
1226+
self.agents: list = []
1227+
1228+
def init_agent(self, agent):
1229+
self.agents.append(agent)
1230+
1231+
1232+
@_needs_sdk_plugins
11531233
@pytest.mark.asyncio
11541234
async def test_a_forwarded_plugin_is_initialized_once_per_thread():
11551235
"""The assertion that outranks the kwarg plumbing.
11561236
1157-
Run against the real ``strands.Agent``: what a plugin is for is the work
1158-
it does in ``init_agent``, and that has to happen against each agent that
1237+
The one test here that needs the SDK's real plugin system, and the reason
1238+
it is worth a skip on older releases: what a plugin is for is the work it
1239+
does in ``init_agent``, and that has to happen against each agent that
11591240
serves requests. Once per thread and against that thread's own agent is
11601241
the whole contract; the kwarg is only how it gets there.
11611242
"""

0 commit comments

Comments
 (0)