fix(teammcp): guard nil provider in handleTeamActionRelayEvents#685
Conversation
📝 WalkthroughWalkthroughThe ChangesProvider Resolution in Relay Events Handler
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
najmuzzaman-mohammad
left a comment
There was a problem hiding this comment.
CodeRabbit CI pass:17, no actionable comments. LGTM.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/teammcp/actions.go (1)
1125-1125:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winApply the same nil-guard pattern here.
This handler still directly accesses
externalActionProvider.GetRelayEventwithout nil checking, causing the same panic risk that was just fixed inhandleTeamActionRelayEvents. Apply the same pattern for consistency.🛡️ Proposed fix
func handleTeamActionRelayEvent(ctx context.Context, _ *mcp.CallToolRequest, args TeamActionRelayEventArgs) (*mcp.CallToolResult, any, error) { + provider, err := selectedActionProvider(action.CapabilityRelayEvents) + if err != nil { + return toolError(err), nil, nil + } - result, err := externalActionProvider.GetRelayEvent(ctx, args.ID) + result, err := provider.GetRelayEvent(ctx, args.ID) if err != nil { return toolError(err), nil, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/teammcp/actions.go` at line 1125, The call to externalActionProvider.GetRelayEvent should be guarded against externalActionProvider being nil to avoid a panic; follow the same nil-guard pattern used in handleTeamActionRelayEvents by first checking if externalActionProvider == nil and returning an appropriate error (or empty response) before calling externalActionProvider.GetRelayEvent, and update the handler around the GetRelayEvent invocation (the variables result, err) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@internal/teammcp/actions.go`:
- Line 1125: The call to externalActionProvider.GetRelayEvent should be guarded
against externalActionProvider being nil to avoid a panic; follow the same
nil-guard pattern used in handleTeamActionRelayEvents by first checking if
externalActionProvider == nil and returning an appropriate error (or empty
response) before calling externalActionProvider.GetRelayEvent, and update the
handler around the GetRelayEvent invocation (the variables result, err)
accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 02c50670-f720-447a-90f0-6fe10ad9d1c5
📒 Files selected for processing (1)
internal/teammcp/actions.go
Summary
handleTeamActionRelayEventsininternal/teammcp/actions.go:747calledexternalActionProvider.ListRelayEventsdirectly, panicking with a nil pointer dereference when no external provider was configured. Every sibling handler routes throughselectedActionProvider, which returns a typed error in that case. This was the only one that didn't.Fix
Resolve the provider via
selectedActionProvider(action.CapabilityRelayEvents)first and surface its error throughtoolError, matching the pattern used by the other relay handlers. TheListRelayEventscall now goes through the resolved provider.Tests
bash scripts/test-go.shpasses.Summary by CodeRabbit