Conversation
WalkthroughVersion bumps in two package files, with a logic update to Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/next-sdk/agent/AgentModelProvider.ts (1)
134-146: Good fix: Correctly prevents closing paired ExtensionClientTransport.The addition of
ExtensionClientTransportto the exclusion list is correct. LikeInMemoryTransportandMessageChannelTransport, it's a paired transport where closing the client side would disrupt the server connection (via Chrome extension messaging). This properly fixes the ReAct mode tool calling failure.Optional: Consider consolidating the two transport exclusion checks
The two separate
ifblocks (lines 136-141 and 144-146) could be combined for better maintainability:- // 如果是 InMemoryTransport 或 MessageChannelTransport,不关闭传输层 - // 因为它们是配对的,关闭一端会影响另一端(服务端) - if ( - (transport && transport instanceof InMemoryTransport) || - (transport && transport instanceof MessageChannelTransport) - ) { - return - } - - // 因为它们是基于 Chrome 扩展的消息传递机制,关闭会影响服务端的连接 - if (transport && transport instanceof ExtensionClientTransport) { - return - } + // 如果是配对传输(InMemoryTransport、MessageChannelTransport、ExtensionClientTransport), + // 不关闭传输层,因为关闭一端会影响另一端(服务端)的连接 + if ( + (transport && transport instanceof InMemoryTransport) || + (transport && transport instanceof MessageChannelTransport) || + (transport && transport instanceof ExtensionClientTransport) + ) { + return + }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/next-remoter/package.jsonpackages/next-sdk/agent/AgentModelProvider.tspackages/next-sdk/package.json
🧰 Additional context used
🧬 Code graph analysis (1)
packages/next-sdk/agent/AgentModelProvider.ts (2)
packages/next-sdk/index.ts (1)
InMemoryTransport(10-10)packages/next-sdk/transport/ExtensionClientTransport.ts (1)
ExtensionClientTransport(11-90)
🔇 Additional comments (3)
packages/next-sdk/package.json (1)
3-3: LGTM: Version bump coordinated with the fix.The beta.2 version increment is appropriate for this bug fix release and aligns with the corresponding version bump in
packages/next-remoter/package.json.packages/next-remoter/package.json (1)
3-3: LGTM: Coordinated version bump.The beta.2 version increment maintains consistency with the next-sdk package update.
packages/next-sdk/agent/AgentModelProvider.ts (1)
143-146: Listener cleanup bypassed for ExtensionClientTransport prevents proper removal.The
close()method in ExtensionClientTransport correctly callsthis._messageListener()to unregister the chrome.runtime.onMessage listener (line 84), but this cleanup is never executed because AgentModelProvider intentionally skips callingclose()on these transports (lines 143-146). This causes listeners to accumulate on repeated transport instantiation cycles, creating a memory leak risk.The current approach assumes transport instances are long-lived, but this assumption should be documented and verified:
- Confirm if ExtensionClientTransport instances are created/destroyed multiple times during a session
- If session/lifecycle events trigger new instances, implement explicit cleanup on those boundaries
- Consider whether the server-connection concerns can be addressed with graceful disconnection instead of skipping cleanup entirely
fix: 修复ReAct模式下调用extensionTransport连接的工具失败问题
Summary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.