fix: move A2AExpressServer to dedicated subpath export for browser compatibility#721
Conversation
The TypeScript SDK moved A2AExpressServer from the ./a2a barrel export to a dedicated ./a2a/express subpath for browser compatibility. See: strands-agents/sdk-typescript#721
|
Issue: The PR description says "Resolves: #718" but #718 appears to be a different PR (log message clarification), not an issue describing the browser compatibility problem. Suggestion: Update the PR description with the correct issue reference, or remove the reference if there's no corresponding issue. If this problem was discovered but not tracked as an issue, consider documenting the root cause more explicitly in the PR description. |
|
Assessment: Request Changes This PR introduces a well-structured fix for browser compatibility by moving Review Categories
The test restructuring and code organization are solid. Please address the documentation PR requirement before merging. |
|
Assessment: Request Changes Thanks for addressing the documentation PR link and issue reference! However, there are a couple of new issues: Review Categories
Once the unrelated file is removed and the description is updated, this looks good to merge. |
b2d8ae6 to
d7c7809
Compare
|
Assessment: Approve All previous feedback has been addressed. The PR now cleanly separates Review Summary
Nice work on the test restructuring to properly validate browser compatibility! |
… into fix/a2a-browser-compat
- Check project test files before starting the A2A Express server - Narrow try/catch to only credential resolution so real failures propagate
|
Assessment: Approve The latest commit "fix: only start A2A server when A2A tests are in scope" addresses zastrowm's concern about the server starting for every test. Response to Open Review ThreadsThread 1 (server starts for every test): ✅ Addressed - setup now checks Thread 2 (browser environment skip): The global setup runs in the parent Node process, not browser. Browser tests connect to the server via HTTP with CORS headers. Thread 3 (when does test skip): The file is I've replied to each thread with detailed explanations. |
aa916f2
Description
The
./a2abarrel export re-exportsA2AExpressServer, which imports Express (a Node-only dependency). This causes bundlers targeting browser environments to fail when importing from@strands-agents/sdk/a2a, even if the consumer only needs browser-safe types likeA2AAgentorA2AServer.This PR moves
A2AExpressServerto a dedicated./a2a/expresssubpath export so the main./a2aentry point remains browser-safe.Related Issues
N/A
Documentation PR
strands-agents/docs#695
Type of Change
Breaking change
Testing
Integration tests were restructured to validate browser compatibility:
a2a-agent.test.tstestsA2AAgentinvoke/stream against a server started in global setup, runs in both Node and browser environmentsexpress-server.test.node.tstests Express-specific functionality (standalone serve, middleware, image file parts), Node-onlyI ran
npm run checkChecklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.