Skip to content

Breaking: fail Worker creation when a StrandsPlugin Activity would replace another Activity - #202

Merged
campoy merged 6 commits into
mainfrom
fix/strands-typescript-same-name-caller-activity
Oct 31, 2026
Merged

campoy merged 6 commits into
mainfrom
fix/strands-typescript-same-name-caller-activity

Conversation

@campoy

@campoy campoy commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What was changed

StrandsPlugin.configureWorker now throws when one of the plugin's Activities would replace a different Activity with the same name:

TypeError: Duplicate registration of Activity '<name>': StrandsPlugin would replace a different Activity with this name. Rename or remove the other Activity, run the other integration on its own Worker and task queue, or use one StrandsPlugin instance on this Worker.
  • Each Activity a StrandsPlugin builds records what it was built from, in a module-level WeakMap: its name, then the models object and default model name (nothing for the default BedrockModel), or the MCP client factory and effective idle timeout. Before super.configureWorker, an own enumerable entry of the Worker's activities with one of the plugin's names is refused unless it is the same function or was built from the same things. The hook and check follow Fail Worker creation when GoogleAdkPlugin would replace another Activity (breaking) #161 for GoogleAdkPlugin; the error text is shared with Refuse a different Activity under an AiSdkPlugin Activity name (breaking) #162 and Breaking: fail Worker creation when an OpenAI Agents plugin Activity would replace another Activity #166.
  • So a caller Activity, another integration's Activity, and a second StrandsPlugin with different options are refused. Setups that are really one plugin still start: the same instance on the NativeConnection and the Worker (where configureWorker runs twice), two instances built from one options object, and two default StrandsPlugin()s. An omitted mcpConnectionIdleTimeout counts as its five-minute default. Two instances with different MCP server names keep both servers.
  • StrandsPlugin.runWorker, which runs after every plugin's configureWorker, refuses an Activity that a plugin applied after StrandsPlugin replaced with a different function: it stops the Worker before it polls, lets it release the connection, and throws Duplicate registration of Activity '<name>': a plugin applied after StrandsPlugin replaced its Activity with this name. with the same remedies. Same check as Refuse a different Activity under an AiSdkPlugin Activity name (breaking) #162.
  • README: names the plugin's Activities, says what fails Worker.create and what fails worker.run(), how to fix it, and when two instances count as the same.

Breaking change: Workers that start today with a collision now fail in Worker.create:

  • a caller Activity named invokeModel, invokeModelStreaming, strandsGetModelConfig, strandsInvokeStatefulModel, strandsInvokeStatefulModelStreaming, <server>-listTools or <server>-callTool;
  • another plugin's different function with one of these names, such as AiSdkPlugin's invokeModel: registered first, it fails Worker.create; applied after StrandsPlugin (later in the Worker's plugins, or on the NativeConnection), it fails worker.run() before the Worker polls;
  • two StrandsPlugins with different options whose Activity names overlap: for example one with models and one with only mcpClients (both register invokeModel), or two factories or idle timeouts for one MCP server name. models objects are compared by identity, so two instances with equal but separate models objects are refused too.

Rename or remove the other Activity, run the other integration on its own Worker and task queue, put every model and MCP client in one StrandsPlugin, or build both from one options object. Workers without a clash behave as before. The title starts with "Breaking:" because release notes come from commit subjects.

Rollback: this changes Worker configuration, not Workflow code. Activity names, inputs, results and History don't change, so recorded histories replay unchanged with this change and without it.

Why?

See #201. The SDK's SimplePlugin merges plugin Activities with { ...existing, ...param }, so a caller Activity with one of the plugin's names was replaced without a warning, and Workflows calling it ran the plugin's handler. The same merge let a second StrandsPlugin replace the first one's Activities: with models on one instance and only mcpClients on a later one, agents that named no model got the second instance's default Bedrock model.

Checklist

  1. Closes [Bug] strands-agents (TypeScript): a plugin Activity silently replaces a caller Activity with the same name #201

  2. How was this tested:

    • New test StrandsPlugin refuses to replace a different Activity with the same name in src/__tests__/test-strands.ts; no existing test covers the plugin's Activity registration. With plugin.ts from main it fails at its first t.throws (configureWorker returns options with the plugin's invokeModel). With this change it passes. It covers the refusals above (a caller Activity, a models plugin then an MCP-only plugin, another models object, another factory or idle timeout for one MCP server) and the setups that still start (the same instance twice, one options object twice, two default instances, an omitted idle timeout next to '5 minutes', different MCP server names).
    • New test runWorker refuses an Activity that a later plugin replaced, before polling: a real Worker with StrandsPlugin and then a plugin registering invokeModel fails run() with the TypeError, ends STOPPED, and its connection.close() succeeds. Without the check, run() polls and never returns; without the release, the state stays INITIALIZED.
    • Locally, in Docker (Node v24.22.0, pnpm 10.27.0, Linux arm64), the steps of _typescript-plugin.yml: make sync, make lint, make test (87 tests pass, after merging main at 34fb981), make build, check_npm_package.py and the clean-consumer smoke install all pass, and scripts/ci/check_conventions.py passes. Prettier reports the changed files clean.
    • [Bug] strands-agents (TypeScript): a plugin Activity silently replaces a caller Activity with the same name #201's reproduction exits 1 (bug) on 1.24.0 and on main at c664a34. With this change, both names fail with the TypeError and it exits 0.
    • A no-server probe of two instances: on 1.24.0 and main, a models plugin then an MCP-only plugin, and two factories for one MCP server, both start with the last instance's Activities. With this change both are refused, while the same instance twice, one options object twice and two default instances start.
    • Real Workers with a local dev server (Temporal CLI 1.9.1, Server 1.32.0; @temporalio/* 1.24.0): a caller invokeModel next to the plugin, and a models plugin with an MCP-only plugin in either order, fail Worker.create, and no handler runs. Without the plugin the caller's Workflow gets its own handler's result, and the models plugin alone answers with its model.

    CI covers the other Node versions and platforms.

  3. Any docs updates needed?
    The README paragraph, included here.

Related: #159/#161, #160/#162 and #165/#166 report and fix the same bug in the other TypeScript plugins. These PRs don't depend on each other. With this PR and #162, AiSdkPlugin and StrandsPlugin on one Worker fail to start in either order, since both register invokeModel and invokeModelStreaming.

Notes:

Draft while the breaking change is reviewed.

@campoy

campoy commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

Updated in 5520af7: a second StrandsPlugin with different options on one Worker is now refused too, as #161 does for GoogleAdkPlugin. Before, the last instance's Activities replaced the first one's; with models on one instance and only mcpClients on the other, agents that named no model got the default Bedrock model. Each plugin Activity now records what it was built from, so the same instance twice, one options object twice and two default instances still start. The test, README and description are updated; owner checks pass locally (34 tests).

…place another Activity

StrandsPlugin merged its Activities over the Worker's `activities`
option. A caller Activity named `invokeModel`, `invokeModelStreaming`,
`<server>-listTools` or `<server>-callTool` was dropped with no error,
and Workflows calling it ran the plugin's handler.

configureWorker now throws a TypeError for such a name. Activities that a
StrandsPlugin built are still replaced, so one instance on both the
NativeConnection and the Worker still starts.

Breaking change: Worker.create now fails when another Activity on the
Worker has the name of a Strands plugin Activity. Rename or remove that
Activity. Activity names, inputs and History do not change.
… Worker

The last StrandsPlugin on a Worker replaced the Activities of an earlier
one. With `models` on one instance and only `mcpClients` on another, the
second instance's default Bedrock model served agents that named no
model, with no error.

Each plugin Activity now records what it was built from: its name, the
`models` object (none for the default), or the MCP factory and idle
timeout. configureWorker refuses to replace an Activity built from
something else, as GoogleAdkPlugin does. The same instance on the
NativeConnection and the Worker, and instances built from one options
object, still start.

Breaking change: Worker.create now also fails for two StrandsPlugins with
different options whose Activity names overlap. Put every model and MCP
client in one StrandsPlugin, or build both from one options object.

@DABH DABH left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review at head d2be784 (two commits over 1e8ae93). On a worktree at the head with pnpm 10.27.0 on PATH: make lint clean, make test 71 of 71 including the new test. Findings were verified with probes against the compiled head: configureWorker chains for each refusal and allowance the body lists, and real Worker.create calls against a local dev server through TestWorkflowEnvironment.createLocal with plugins on its NativeConnection. Checked and sound: the own-enumerable test matches exactly what SimplePlugin's { ...existing, ...param } merge replaces; the same instance on the NativeConnection and the Worker, one options object twice, two default instances, a default plus an MCP-only instance, and an omitted idle timeout next to '5 minutes' all start; Replayers are unaffected because configureReplayWorker merges no Activities, so the rollback claim holds; strandsGetModelConfig is covered, and #285 adds its two stateful Activities to the same object literal ahead of the sources loop (plugin.ts:99), so they are covered after a rebase with no code change; #200 adds no Activities and only touches the same @temporalio/worker import line. Both outcomes the body flags are reasonable: model factories are opaque closures, so identity is the only equality that cannot bring back a silent replacement, and before this change an AiSdkPlugin next to a StrandsPlugin silently ran one integration's invokeModel for the other's Workflows with the wrong input shape, so failing at start-up is strictly better. Not verified: the real AiSdkPlugin (not installed in this plugin's environment; a stand-in SimplePlugin registering the same two names was used), and the macOS, Windows and Node 22 cells beyond CI, which is green.

Severity: P1 = wrong result or stuck run in realistic use; P2 = correctness or robustness in an edge case; P3 = docs, tests, cleanup.

Where the problems cluster: the check is correct in every case probed; all three items are places where the README, the JSDoc or the error message describe the rule less precisely than the code applies it. Approving because no P1 or P2 was found, CI is green, and the three P3 items below are non-blocking.

Outside this PR, not blocking and possibly worth an issue: the check is per Worker, but the MCP connection cache is process-wide and keyed by server name only (temporal-mcp-client.ts:216, read in acquireConnection at 238-264). Two Workers in one process with different factories for the same server name share whichever connection opened first (probe: the second plugin's s-listTools returned the first factory's tools; same on main), and either Worker's shutdown evicts it (plugin.ts:122-124).

Comment thread typescript/strands-agents/README.md Outdated

The read is a new activity, so a run whose agent ran before you deploy this version fails to replay on it with a nondeterminism error. Let those runs finish first, or use Worker Versioning.

The plugin's activities are named `invokeModel`, `invokeModelStreaming`, and `{name}-listTools` and `{name}-callTool` for each MCP client. If the worker already has a different activity with one of these names (your own activity, another integration's activity, or one from a second `StrandsPlugin` with different options), creating the worker fails with `Duplicate registration of Activity '<name>'`. Rename your activity, run the other integration on its own worker, or put every model and MCP client in one `StrandsPlugin`. The same instance on the `NativeConnection` and the worker, or two instances built from the same options object, still work.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 (non-blocking): the list of the plugin's activity names leaves out strandsGetModelConfig, which the check also refuses. A caller activities: { strandsGetModelConfig: async () => 1 } next to new StrandsPlugin({ models: { stub } }) throws Duplicate registration of Activity 'strandsGetModelConfig' (probe against the compiled head), and the paragraph two above this one names that activity. The PR body's breaking-change list has the same gap. #285 adds strandsInvokeStatefulModel and strandsInvokeStatefulModelStreaming to the same object literal ahead of the sources loop, so the check covers them with no code change, but whichever PR lands second should add them here. Fix: "The plugin's activities are named invokeModel, invokeModelStreaming, strandsGetModelConfig, and {name}-listTools and {name}-callTool for each MCP client", and add the name to the body's list.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in 14315e6. The README list now names strandsGetModelConfig, and also strandsInvokeStatefulModel and strandsInvokeStatefulModelStreaming, since #285 landed first (merged into this branch in 008e7a3). The check covers them with no code change. I'm adding them to the breaking-change list in the PR description too.

Comment thread typescript/strands-agents/src/plugin.ts Outdated
if (typeof other === 'function' && other !== fn && !sameSources(other, fn)) {
throw new TypeError(
`Duplicate registration of Activity '${name}': StrandsPlugin would replace a different Activity ` +
'with this name. Rename or remove the other registration, or give each StrandsPlugin on this Worker ' +

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 (non-blocking): the message tells the user to "give each StrandsPlugin on this Worker the same options", but equal options are refused unless they share the models object and the MCP factory functions, so someone who hits the identity rule is told to do what they already did. new StrandsPlugin({ models: { stub } }) twice, with the same stub function, throws this message for invokeModel. A realistic way to get there: TestWorkflowEnvironment.createLocal({ plugins: [new StrandsPlugin({ models: { test: () => stub } })] }) passes the plugins to env.nativeConnection as well as the client, and Worker.create({ connection: env.nativeConnection, plugins: [new StrandsPlugin({ models: { test: () => stub } })] }) then fails with this TypeError against a local dev server; the same two calls with one shared models constant start. README line 98 says "a second StrandsPlugin with different options" and has the same gap. Identity is a sound rule; the wording should state it. Suggested message tail: ...or register one StrandsPlugin instance, or build each from the same models object and MCP client factories. README: "Two instances count as the same only when they share the models object (or both omit it) and, for each MCP server name they share, the same factory function and idle timeout. Two equal object literals are refused." The text is shared with #161 and #162, so change it in all three or in none. Optional: compare models per key (same keys, each factory ===, same derived default) instead of by object; ModelActivity only reads the factories and the default name (model-activity.ts:84-108), so that would accept { models: { stub } } twice and still refuse inline arrow factories.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, changed in 14315e6. The message now ends: "Rename or remove the other Activity, run the other integration on its own Worker and task queue, or use one StrandsPlugin instance on this Worker." #162 and #166 use the same remedies. The README now states the identity rule: "Two instances count as the same only when they share the models object (or both omit it) and, for each MCP server name they share, the same factory function and idle timeout, as when both are built from one options object. Two equal object literals are refused."

I kept the comparison by models object rather than per key. Per key only helps when the factories are shared but the models literal isn't. Your TestWorkflowEnvironment example uses inline factories, which a per-key compare would still refuse.

#161 was merged with the old text before this round, so GoogleAdkPlugin would need a small follow-up to match.

Comment thread typescript/strands-agents/src/plugin.ts Outdated
* Adds the model and MCP Activities to the Worker.
*
* @throws TypeError if the Worker already has a different Activity with one of these names: a
* caller Activity, another integration's Activity, or one from a `StrandsPlugin` with different

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 (non-blocking): "another integration's Activity" is refused only when that integration is registered before StrandsPlugin; a later plugin still replaces these Activities silently, and neither this JSDoc nor README line 98 says so. Worker.create applies the Worker's plugins in order and then connection.plugins (@temporalio/worker 1.24.0 worker.js:196-201). With a stand-in SimplePlugin registering invokeModel and invokeModelStreaming (the names AiSdkPlugin registers, vercel-ai-sdk/src/activities.ts:240,255): [new StrandsPlugin(), other] starts and invokeModel is the other plugin's function; with other on TestWorkflowEnvironment's NativeConnection and StrandsPlugin on the Worker, a real Worker.create starts and worker.options.activities.get('invokeModel') is the other function; [other, new StrandsPlugin()] is refused. The body says this ("registered first"; #162 closes the AiSdkPlugin case in both orders), but someone reading the README would not know it, and until #162 lands plugins: [new StrandsPlugin(), new AiSdkPlugin()] starts. Fix: add to README line 98 and to this JSDoc: "The check sees only activities registered before StrandsPlugin: the worker's own activities and plugins earlier in the worker's plugins. A plugin later in that list, or any plugin on the NativeConnection (applied after the worker's plugins), can still replace these activities unless it checks too."

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, done in 373a9a6, and it goes one step further than docs. StrandsPlugin now also overrides runWorker, which runs after every plugin's configureWorker. If a plugin applied after it replaced one of its Activities with a different function, it stops the Worker before it polls, lets it release the connection, and throws Duplicate registration of Activity '<name>': a plugin applied after StrandsPlugin replaced its Activity with this name., followed by the same remedies. This is the same check #162 added in 58c9ca2, so a later plugin, including one on the NativeConnection, no longer slips through in either PR.

The README now says: "If it is registered before StrandsPlugin (in the worker's activities or by an earlier plugin), creating the worker fails. If a plugin applied after StrandsPlugin replaces one, worker.run() fails before the worker polls. Worker.create applies the worker's plugins in order, then the NativeConnection's." The runWorker JSDoc says the same.

The new test, runWorker refuses an Activity that a later plugin replaced, before polling, runs a real Worker with [new StrandsPlugin(...), laterPlugin] and checks the TypeError, that the Worker ends STOPPED, and that connection.close() succeeds. Without the check, run() starts polling and never returns. Without the release, the state stays INITIALIZED.

A check in SimplePlugin's merge would still be the general fix for every plugin, but that needs an SDK change.

…duplicate error

The README now names strandsGetModelConfig and the two stateful model
Activities, and says when two StrandsPlugin instances count as the same.
The error's remedies no longer say "the same options", since instances
are compared by object identity.
… replaced

configureWorker sees only Activities registered before StrandsPlugin, so
a plugin applied after it could still replace one silently. runWorker
runs after every plugin's configureWorker: if one of the plugin's
Activities was replaced by a different function, it stops the Worker
before it polls, lets it release the connection, and throws.
@campoy

campoy commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review, @DABH. All three P3s are addressed, in 14315e6 (README names, error remedies and the identity rule) and 373a9a6 (a runWorker check for plugins applied later, as in #162). There's a reply in each thread. I also merged main twice to clear conflicts with #200 and #285; the diff against main is otherwise unchanged.

On the MCP connection cache: agreed, it's outside this PR and also on main. I've noted it to file separately.

@campoy
campoy merged commit 283e185 into main Oct 31, 2026
27 checks passed
@campoy
campoy deleted the fix/strands-typescript-same-name-caller-activity branch October 31, 2026 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] strands-agents (TypeScript): a plugin Activity silently replaces a caller Activity with the same name

2 participants