Skip to content

fix missing params for upstream.MCPNamein filtered_tools_handler - #1180

Merged
maleck13 merged 1 commit into
Kuadrant:mainfrom
aniketpandey05:fix/mcpname-missing-parens
Jun 24, 2026
Merged

fix missing params for upstream.MCPNamein filtered_tools_handler#1180
maleck13 merged 1 commit into
Kuadrant:mainfrom
aniketpandey05:fix/mcpname-missing-parens

Conversation

@aniketpandey05

@aniketpandey05 aniketpandey05 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes incorrect logging in filtered_tools_handler.go by invoking upstream.MCPName() instead of referencing the method value. This ensures the actual upstream MCP server name is included in log messages, improving observability and debugging.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed incorrect method invocation in debug logging for tool filtering, improving log accuracy for troubleshooting and debugging.

Signed-off-by: rogueslasher <aniketpandey25092005@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 59fccafe-d22a-405a-bdff-e8523c9c0e49

📥 Commits

Reviewing files that changed from the base of the PR and between b8dd4c2 and 5516798.

📒 Files selected for processing (1)
  • internal/broker/filtered_tools_handler.go

📝 Walkthrough

Walkthrough

Single-character fix in filterToolsByServerMap: the debug log for "no tools registered for upstream server" now calls upstream.MCPName() as a method instead of referencing upstream.MCPName as a field.

Changes

Fix MCPName() method call in debug log

Layer / File(s) Summary
Fix MCPName() invocation in debug log
internal/broker/filtered_tools_handler.go
Debug log now invokes upstream.MCPName() correctly instead of referencing upstream.MCPName without parentheses.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Suggested labels

review-effort/small

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing a missing function invocation on upstream.MCPName in filtered_tools_handler.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aniketpandey05

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot added the review-effort/small Low review effort (1-2): straightforward, single file, config/docs label Jun 22, 2026
@aniketpandey05
aniketpandey05 marked this pull request as ready for review June 22, 2026 17:14
@aniketpandey05

Copy link
Copy Markdown
Contributor Author

@david-martin @jasonmadigan @maleck13 this one can be merged too it fixes the missing ()

@github-actions

Copy link
Copy Markdown
Contributor

Thanks for the contribution, @rogueslasher! You currently have other non-draft PR(s) open:

To help us review and merge changes as efficiently as possible, we ask contributors to focus on one PR at a time. Activity on this project can be high, and maintainers have other priorities outside the project, so having a single active PR helps everyone get changes landed faster.

Please convert this PR to draft while your other PR(s) are in review. Once they are merged or closed, mark this one as ready for review and we will take a look.

Note

This is an experimental process and may change or need manual intervention while we trial it.

@Aman-Cool

Copy link
Copy Markdown
Collaborator

@rogueslasher, Confirmed.., upstream.MCPName was passing the method value instead of calling it, so the line logged a func, not the name. Grepped the tree too: this is the only instance left, everything else already calls MCPName().

Optional: you can actually drop the call here. serverName is already in scope from the loop above, and since findServerByName only returns the upstream whose MCPName() == serverName, the two are equal.., so "server", serverName does the same thing without the call, and matches the Error log just above. Fine to leave as-is though.

@maleck13
maleck13 merged commit feb3d96 into Kuadrant:main Jun 24, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review-effort/small Low review effort (1-2): straightforward, single file, config/docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants