Skip to content
This repository was archived by the owner on Jun 3, 2026. It is now read-only.

fix: validate agent implements stream() in AgentNode constructor#710

Closed
jsamuel1 wants to merge 1 commit into
strands-agents:mainfrom
jsamuel1:fix/agent-node-validation
Closed

fix: validate agent implements stream() in AgentNode constructor#710
jsamuel1 wants to merge 1 commit into
strands-agents:mainfrom
jsamuel1:fix/agent-node-validation

Conversation

@jsamuel1

Copy link
Copy Markdown
Contributor

Problem

Passing an object that doesn't implement InvokableAgent.stream() to AgentNode (or indirectly via Graph/Swarm node definitions) produces a cryptic runtime error:

_ is not a function

This happens because _resolveNodes() casts unrecognized objects to InvokableAgent without validation, and the error only surfaces later when AgentNode.handle() calls this._agent.stream().

Solution

Add an early check in AgentNode constructor that verifies the agent has a stream() method. The error message includes the node id for easier debugging:

node_id=<marge> | agent does not implement stream(). Ensure the object passed as an agent has a stream() method.

Testing

  • Added unit test for the validation
  • All existing tests pass (1633 tests, 0 failures)

Comment thread src/multiagent/nodes.ts Outdated
Comment thread src/multiagent/__tests__/nodes.test.ts Outdated

@JiwaniZakir JiwaniZakir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The validation in nodes.ts checks typeof agent.stream !== 'function' before calling super(), which is correct, but it then references agent.id in the error message without first verifying that agent.id is defined. If a caller passes an object with neither stream nor id, the error reads node_id=<undefined>, which is less helpful than it could be. A simple fallback like agent.id ?? 'unknown' would make the message more robust.

The test in nodes.test.ts is placed before the beforeEach block, which is unconventional — it inverts the typical setup-then-test structure and could confuse readers scanning the file. Moving it after beforeEach (or into a dedicated describe('constructor validation', ...) block) would be cleaner.

The test itself only covers the case where id is present but stream is absent. Adding a case where neither property exists would exercise the agent.id undefined path mentioned above and ensure the error message degrades gracefully in that scenario.

@jsamuel1 jsamuel1 force-pushed the fix/agent-node-validation branch from ddbbabb to 19d5964 Compare March 27, 2026 12:01
@jsamuel1 jsamuel1 temporarily deployed to manual-approval March 27, 2026 12:01 — with GitHub Actions Inactive
@jsamuel1 jsamuel1 temporarily deployed to manual-approval March 27, 2026 12:01 — with GitHub Actions Inactive
@jsamuel1

Copy link
Copy Markdown
Contributor Author

Thanks @JiwaniZakir — all three points addressed:

  • Added agent.id ?? 'unknown' fallback so the error message degrades gracefully
  • Moved the test into a describe('constructor') block after beforeEach
  • Added a test case for when neither stream nor id exists

@JiwaniZakir

Copy link
Copy Markdown

The fix is straightforward and the error message including node_id is a nice touch for debuggability. One consideration: since stream could technically be a non-function property, the check should use typeof this._agent.stream === 'function' rather than just checking existence — worth confirming that's what the implementation does. Otherwise this is a clean improvement over the silent runtime failure.

@zastrowm

Copy link
Copy Markdown
Member

@jsamuel1 out of curiosity, how did you encounter this? Are you using JS directly?

@github-actions github-actions Bot added strands-running <strands-managed> Whether or not an agent is currently running and removed strands-running <strands-managed> Whether or not an agent is currently running labels Mar 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Assessment: Approve

Clean, focused bug fix that improves developer experience by catching invalid AgentNode inputs early with a clear error message instead of the cryptic "_ is not a function" error.

Review Summary
  • Code Quality: Implementation correctly uses typeof agent.stream !== 'function' and error message follows the SDK logging format
  • Testing: Two test cases properly cover both scenarios (with/without agent id), following repository patterns
  • Documentation: Not required for this bug fix

All prior review feedback has been addressed. Nice improvement for debuggability 👍

@JiwaniZakir

Copy link
Copy Markdown

The duplicate PR (#524) being stuck in review for 1.5 months is worth resolving before merging this one — either close #524 in favor of this or consolidate. On the implementation, the typeof this._agent.stream === 'function' check @JiwaniZakir flagged is the correct approach and worth confirming is in place, since a property named stream with a non-function value would otherwise pass the guard and still produce a runtime error.

@strands-agents strands-agents deleted a comment from github-actions Bot Mar 27, 2026
@zastrowm

Copy link
Copy Markdown
Member

The duplicate PR (#524) being stuck in review for 1.5 months is worth resolving before merging this one — either close #524 in favor of this or consolidate.

That PR does not seem like a duplicate of this one

... @JiwaniZakir ...

That that you're calling out your own comment in the third person make me feel like this is an automated response. While we appreciate feedback from the community, we do have our own review agents and would like human feedback instead of agentic; please keep that in mind.

@JiwaniZakir

This comment was marked as spam.

Add early validation in AgentNode to check that the provided agent
object has a stream() method. Without this, passing an object that
doesn't implement InvokableAgent.stream() produces a cryptic
'_ is not a function' error at execution time.

The check runs at construction time so the error is immediate and
the message includes the node id for easier debugging.

---
Prompt: I had an error in my app using this library: The error _ is
not a function with node_id=<marge>
@jsamuel1 jsamuel1 force-pushed the fix/agent-node-validation branch from 19d5964 to dd35183 Compare April 11, 2026 02:10
@strands-agent

Copy link
Copy Markdown
Collaborator

This repository has been merged into the strands-agents/harness-sdk monorepo and will be archived shortly. All new development happens there.

If this PR is still relevant, please recreate it against the monorepo. The code now lives under strands-ts/. Full commit history was preserved, so your base should be findable.

Apologies for the disruption, and thank you for contributing!

@pgrayy pgrayy closed this Jun 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants