Skip to content

Commit 7119f8e

Browse files
ranst91hiepcs
andcommitted
fix(aws-strands): forward plugins per-thread and warn on agent-bound params
The adapter rebuilds a fresh strands.Agent per thread_id by reading the template's constructor params back off the instance. Strands consumes `plugins` during init: it runs each plugin's init_agent against the agent that received it, registers that plugin's hooks and tools into that agent's registries, and keeps only a _PluginRegistry holding a weakref back to the agent. _references_agent sees that weakref and classifies `plugins` as template-owned, and template-owned params were deliberately excluded from the uncarried-param warning. A caller who set plugins on the template therefore got neither the plugins nor a word about losing them. The template never serves a request, so a plugin whose behaviour lives in init_agent silently did nothing. Report both kinds of uncarried param, each with its own message and both under the existing once-per-param bookkeeping. They stay separate because they point at different fixes: an unreadable param is an adapter gap a later release may close, while a param the SDK wired to one agent will never be carryable. Add the `plugins=` kwarg Python was missing, matching StrandsAgentOptions.plugins on the TypeScript side. It forwards into every per-thread StrandsAgentCore next to `hooks`, under the same falsy-omission rule, and "plugins" joins _AGUI_EXPLICIT_PARAMS so it is not also probed off the template. Since that exclusion hides it from the generic probe, a dedicated read keeps the template case reportable, filtering out the plugins Strands registers on every Agent itself so the warning does not reach callers who set none. Precedence is unchanged: thread_agent_kwargs wins at the merge site, then the explicit kwarg, and the template never carries. The warning stays quiet for a thread whose kwargs supplied the param either way. Also drops a duplicated _report_uncarried_params call at the build site that was a no-op given the once-per-param bookkeeping. The forwarding half was proposed in #2141 by @hiepcs against an older tree. Co-authored-by: HiepBP <hiepcs@users.noreply.github.com>
1 parent 33b1caf commit 7119f8e

3 files changed

Lines changed: 477 additions & 27 deletions

File tree

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

Lines changed: 134 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,10 @@
4444
# "session_manager" is excluded: it is supplied per-thread via
4545
# StrandsAgentConfig.session_manager_provider (see run()). Forwarding a
4646
# template-level session_manager would make every thread share one session_id.
47+
# "plugins" is excluded: Agent consumes the list during init, registering each
48+
# plugin's hooks and tools into its own registries and keeping only a registry
49+
# bound to that agent, so there is no list to read back. Callers supply them
50+
# per-thread through the explicit StrandsAgent(plugins=...) kwarg.
4751
_AGUI_EXPLICIT_PARAMS = {
4852
"self",
4953
"model",
@@ -52,6 +56,7 @@
5256
"messages",
5357
"hooks",
5458
"session_manager",
59+
"plugins",
5560
}
5661

5762

@@ -171,6 +176,52 @@ def _read(prop: property) -> Any:
171176
return _MISSING
172177

173178

179+
# Strands namespaces the plugins it registers on every Agent itself, and
180+
# registers them whether or not the caller passed any. Anything under this
181+
# prefix is therefore the SDK's, not a setting to report as dropped.
182+
_SDK_PLUGIN_NAME_PREFIX = "strands:"
183+
184+
185+
def _template_plugin_names(agent: Any) -> List[str]:
186+
"""Names of the plugins the caller put on the template.
187+
188+
``plugins`` is handled through an explicit kwarg, so the generic probe
189+
skips it and would never report it. Reading the registry here is what
190+
lets a caller who set plugins on the template be told they do not carry,
191+
instead of getting silence.
192+
193+
Strands' own plugins are filtered out by name. Every Agent is built with
194+
at least one of them, so counting them would warn every caller about a
195+
setting nobody made. A caller plugin that borrowed the SDK's prefix would
196+
be missed by this, which is the harmless direction: the cost is one
197+
warning not said, against a warning said to everyone.
198+
"""
199+
for attr in _candidate_attributes("plugins"):
200+
try:
201+
holder = getattr(agent, attr, None)
202+
except Exception: # noqa: BLE001 - a raising property is not a plugin list
203+
continue
204+
if holder is None:
205+
continue
206+
if isinstance(holder, (list, tuple)):
207+
contents: Any = holder
208+
else:
209+
contents = _registry_contents(holder)
210+
if contents is _MISSING or not contents:
211+
continue
212+
names = []
213+
for plugin in contents:
214+
name = getattr(plugin, "name", None)
215+
# An entry with no readable name cannot be attributed to the SDK,
216+
# so it counts as the caller's rather than being dropped silently.
217+
label = name if isinstance(name, str) else type(plugin).__name__
218+
if not label.startswith(_SDK_PLUGIN_NAME_PREFIX):
219+
names.append(label)
220+
if names:
221+
return names
222+
return []
223+
224+
174225
def _element_type(annotation: Any) -> Any:
175226
"""The element type of a ``list[X]``-shaped annotation, or ``None``.
176227
@@ -2997,6 +3048,7 @@ def __init__(
29973048
description: str = "",
29983049
config: "StrandsAgentConfig | None" = None,
29993050
hooks: "list | None" = None,
3051+
plugins: "list | None" = None,
30003052
agents_by_thread: "Dict[str, Any] | None" = None,
30013053
):
30023054
# Detect a multi-agent orchestrator structurally. A Graph or Swarm has
@@ -3065,9 +3117,18 @@ def __init__(
30653117
self._unreadable_params = []
30663118
self._template_owned_params = []
30673119

3068-
# Params wired to the template are a known structural limit, not a
3069-
# surprise, so they are recorded without a warning. Params this adapter
3070-
# could not read at all are the ones worth interrupting for.
3120+
# ``plugins`` is handled explicitly, so the generic probe above skips
3121+
# it and cannot report it. A template built with plugins is still a
3122+
# dropped setting, so record it here and let it be reported through the
3123+
# same route as every other param that will not carry.
3124+
if self._orchestrator is None and _template_plugin_names(agent):
3125+
self._template_owned_params.append("plugins")
3126+
3127+
# Both kinds of param will fail to reach per-thread agents, and both
3128+
# are reported when a thread is built. They are kept apart because they
3129+
# ask for different reading: an unreadable param is a gap in this
3130+
# adapter that a later release may close, while one the SDK wired to
3131+
# the agent that received it will never carry.
30713132
self._unforwardable_params = [
30723133
*self._unreadable_params,
30733134
*self._template_owned_params,
@@ -3096,6 +3157,20 @@ def __init__(
30963157
# observability / loop-cap / policy-enforcement hook actually fires.
30973158
self._hooks = list(hooks) if hooks else []
30983159

3160+
# Plugins forwarded to each per-thread StrandsAgentCore.
3161+
#
3162+
# A dedicated kwarg for the same reason ``hooks`` has one, one step
3163+
# further along. Strands consumes the plugin list during init: it calls
3164+
# each plugin's ``init_agent`` and registers its hooks and tools into
3165+
# that agent's registries, keeping only a registry bound to that agent.
3166+
# There is no list left to read back, and the registry cannot be handed
3167+
# to a second agent. Since the template never serves a request, a
3168+
# plugin registered there never runs against the agents that do, and a
3169+
# plugin whose whole behaviour lives in ``init_agent`` silently does
3170+
# nothing. Taking them from the caller instead lets every per-thread
3171+
# agent build its own.
3172+
self._plugins = list(plugins) if plugins else []
3173+
30993174
self.name = name
31003175
self.description = description
31013176
self.config = config or StrandsAgentConfig()
@@ -3580,25 +3655,57 @@ def _report_uncarried_params(self, core_kwargs: dict) -> None:
35803655
Said once per param, and only about params this thread's kwargs did not
35813656
supply, so acting on it makes it stop without the first thread becoming
35823657
the policy for every later one.
3658+
3659+
The two kinds get their own message. An unreadable param is a gap in
3660+
this adapter, and a caller reading that can reasonably wait for a later
3661+
release to close it. A param the SDK wired to the agent that received
3662+
it is a structural limit rather than a gap: no adapter release will
3663+
carry it, so the per-thread route is the whole answer rather than a
3664+
stopgap. One sentence for both would send half the readers after a fix
3665+
that is not coming.
35833666
"""
3584-
still_missing = sorted(
3585-
name
3586-
for name in self._unreadable_params
3587-
if name not in core_kwargs and name not in self._reported_uncarried
3588-
)
3589-
if not still_missing:
3590-
return
3591-
self._reported_uncarried.update(still_missing)
3592-
# Phrased as a capability, not an accusation: an unreadable param is
3593-
# unreadable whether or not the caller set one, so this cannot say that
3594-
# anything was actually lost.
3595-
logger.warning(
3596-
"this Strands release stores these Agent constructor params where the "
3597-
"adapter cannot read them back, so a value set on the template through "
3598-
"them will not reach per-thread agents: %s. Supply them per thread "
3599-
"with StrandsAgentConfig.thread_agent_kwargs.",
3600-
", ".join(still_missing),
3601-
)
3667+
3668+
def _unreported(names: List[str]) -> List[str]:
3669+
return sorted(
3670+
name
3671+
for name in names
3672+
if name not in core_kwargs and name not in self._reported_uncarried
3673+
)
3674+
3675+
unreadable = _unreported(self._unreadable_params)
3676+
template_owned = _unreported(self._template_owned_params)
3677+
self._reported_uncarried.update(unreadable)
3678+
self._reported_uncarried.update(template_owned)
3679+
3680+
if unreadable:
3681+
# Phrased as a capability, not an accusation: an unreadable param
3682+
# is unreadable whether or not the caller set one, so this cannot
3683+
# say that anything was actually lost.
3684+
logger.warning(
3685+
"this Strands release stores these Agent constructor params where the "
3686+
"adapter cannot read them back, so a value set on the template through "
3687+
"them will not reach per-thread agents: %s. Supply them per thread "
3688+
"with StrandsAgentConfig.thread_agent_kwargs.",
3689+
", ".join(unreadable),
3690+
)
3691+
if template_owned:
3692+
# ``plugins`` is the one of these with a dedicated kwarg, so point
3693+
# at it rather than making every caller write a hook for the case
3694+
# the adapter already has an answer to.
3695+
route = (
3696+
"Pass them to StrandsAgent(plugins=[...])"
3697+
if template_owned == ["plugins"]
3698+
else "Supply them per thread with "
3699+
"StrandsAgentConfig.thread_agent_kwargs"
3700+
)
3701+
logger.warning(
3702+
"these Agent constructor params are consumed by the Strands Agent "
3703+
"that received them and cannot be handed to another agent, so a "
3704+
"value set on the template will not reach per-thread agents: %s. "
3705+
"%s.",
3706+
", ".join(template_owned),
3707+
route,
3708+
)
36023709

36033710
async def run(
36043711
self,
@@ -3825,6 +3932,12 @@ async def _run_raw(
38253932
core_kwargs = dict(self._agent_kwargs)
38263933
if self._hooks:
38273934
core_kwargs["hooks"] = list(self._hooks)
3935+
# Same falsy-omission rule as hooks, for the same reason:
3936+
# ``plugins=[]`` is a value a future StrandsAgentCore could
3937+
# read as "disable the defaults", which is not what an
3938+
# absent setting means.
3939+
if self._plugins:
3940+
core_kwargs["plugins"] = list(self._plugins)
38283941
# The caller's per-thread kwargs go on last, so they can
38293942
# supply what the template cannot carry and override what
38303943
# it can. See StrandsAgentConfig.thread_agent_kwargs.
@@ -3854,8 +3967,6 @@ async def _run_raw(
38543967
return
38553968
core_kwargs.update(dict(extra or {}))
38563969
self._report_uncarried_params(core_kwargs)
3857-
if self.config.thread_agent_kwargs is None:
3858-
self._report_uncarried_params(core_kwargs)
38593970
# Re-asserted after the caller: these keep threads apart
38603971
# and a run coherent, so they stay the adapter's to set.
38613972
for owned in ("model", "system_prompt", "tools", "session_manager"):

0 commit comments

Comments
 (0)