fix: validate agent implements stream() in AgentNode constructor#710
fix: validate agent implements stream() in AgentNode constructor#710jsamuel1 wants to merge 1 commit intostrands-agents:mainfrom
Conversation
JiwaniZakir
left a comment
There was a problem hiding this comment.
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.
ddbbabb to
19d5964
Compare
|
Thanks @JiwaniZakir — all three points addressed:
|
|
The fix is straightforward and the error message including |
|
@jsamuel1 out of curiosity, how did you encounter this? Are you using JS directly? |
|
Assessment: Approve Clean, focused bug fix that improves developer experience by catching invalid Review Summary
All prior review feedback has been addressed. Nice improvement for debuggability 👍 |
|
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 |
That PR does not seem like a duplicate of this one
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. |
This comment was marked as spam.
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>
19d5964 to
dd35183
Compare
Problem
Passing an object that doesn't implement
InvokableAgent.stream()toAgentNode(or indirectly viaGraph/Swarmnode definitions) produces a cryptic runtime error:This happens because
_resolveNodes()casts unrecognized objects toInvokableAgentwithout validation, and the error only surfaces later whenAgentNode.handle()callsthis._agent.stream().Solution
Add an early check in
AgentNodeconstructor that verifies the agent has astream()method. The error message includes the node id for easier debugging:Testing