Skip to content

Conversation

@furysama
Copy link
Contributor

@furysama furysama commented Dec 10, 2025

Description

ListResources can return null when provided with no MCP resources. Certain MCP clients will fail in this case. This PR returns an empty slice instead, which should be safer.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • MCP spec compatibility implementation
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Code refactoring (no functional changes)
  • Performance improvement
  • Tests only (no functional changes)
  • Other (please describe):

Checklist

  • My code follows the code style of this project
  • I have performed a self-review of my own code
  • I have added tests that prove my fix is effective or that my feature works
  • I have updated the documentation accordingly

Additional Information

Summary by CodeRabbit

  • Bug Fixes
    • Prevented an error when listing resources by ensuring empty results are handled safely, improving stability when processing empty or missing data.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Walkthrough

Adds a nil-check in handleListResources to ensure resourcesToReturn is non-nil by assigning an empty slice when it would otherwise be nil, preventing potential nil-pointer issues when constructing the ListResourcesResult.

Changes

Cohort / File(s) Summary
Nil-check for resources slice
server/server.go
Adds a guard that assigns an empty slice to resourcesToReturn when it is nil in handleListResources, avoiding nil-pointer issues during result construction.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Attention: verify the nil-check targets the correct variable name and that the empty-slice assignment integrates correctly with subsequent logic.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: fixing a nil resources slice issue, which matches the PR's primary objective.
Description check ✅ Passed The description covers the bug, its impact, and the solution. Type of change is marked, and the checklist is mostly completed with relevant items checked.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between df2e869 and 2a0522b.

📒 Files selected for processing (1)
  • server/server.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • server/server.go

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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1f07bf3 and df2e869.

📒 Files selected for processing (1)
  • server/server.go (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go

📄 CodeRabbit inference engine (AGENTS.md)

**/*.go: Order imports: standard library first, then third-party, then local packages (goimports enforces this)
Follow Go naming conventions: exported identifiers in PascalCase; unexported in camelCase; acronyms uppercase (HTTP, JSON, MCP)
Error handling: return sentinel errors, wrap with fmt.Errorf("context: %w", err), and check with errors.Is/As
Prefer explicit types and strongly-typed structs; avoid using any except where protocol flexibility is required (e.g., Arguments any)
All exported types and functions must have GoDoc comments starting with the identifier name; avoid inline comments unless necessary
Functions that are handlers or long-running must accept context.Context as the first parameter
Ensure thread safety for shared state using sync.Mutex and document thread-safety requirements in comments
For JSON: use json struct tags with omitempty for optional fields; use json.RawMessage for flexible/deferred parsing

Files:

  • server/server.go
🧠 Learnings (2)
📓 Common learnings
Learnt from: ezynda3
Repo: mark3labs/mcp-go PR: 461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.
📚 Learning: 2025-06-30T07:13:17.052Z
Learnt from: ezynda3
Repo: mark3labs/mcp-go PR: 461
File: server/sampling.go:22-26
Timestamp: 2025-06-30T07:13:17.052Z
Learning: In the mark3labs/mcp-go project, the MCPServer.capabilities field is a struct value (serverCapabilities), not a pointer, so it cannot be nil and doesn't require nil checking. Only pointer fields within the capabilities struct should be checked for nil.

Applied to files:

  • server/server.go
🧬 Code graph analysis (1)
server/server.go (1)
mcp/types.go (1)
  • Resource (715-734)

@ezynda3 ezynda3 merged commit bd8762d into mark3labs:main Dec 19, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants