Skip to content

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

Open
jsamuel1 wants to merge 1 commit intostrands-agents:mainfrom
jsamuel1:fix/agent-node-validation
Open

fix: validate agent implements stream() in AgentNode constructor#710
jsamuel1 wants to merge 1 commit intostrands-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)

Copy link
Copy Markdown

@JiwaniZakir JiwaniZakir left a comment

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

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
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.

4 participants