-
Notifications
You must be signed in to change notification settings - Fork 7.6k
fix: align everything server resource URI numbering with array indices #2570
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Fixes issue where array indices (0-based) didn't match URI numbers (1-based), causing confusion where: - Resource at index 0 was accessed via 'test://static/resource/1' - Even indices (text resources) mapped to odd URI numbers Changes: - Resource URIs now use 0-based numbering to match array indices - Resource 0 is now accessed via 'test://static/resource/0' - Even indices now map to even URI numbers (more intuitive) Resolves #475 🤖 Generated with [Claude Code](https://claude.ai/code) Co-authored-by: Ola Hungerford <olaservo@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
I am a bit confused why we used 1-indexing in the first place, so am a bit wary (something something Chesterton's Fence). But seems fairly low risk.
@domdomegg I'm guessing it was so that in the Inspector when you look at the resource list it is human-aligned, i.e., only programmers would not scratch their head at "Resource 0". Guess we'll never really know, since this was part of the Initial Commit for the repo, and there is no associated detail in the commit message. ![]() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what we had. The URI matched the resource name and resources started at 1 for human readability.
Now you have name out of sync with URI. The array index matches the URI, but is there a clear win in that? In the real world, URI and array index may have no correlation. I see no clear benefit to this change. You could make the resource name match the zero-based URI and index, I suppose, but I still don't see how it's an improvement that needs to be made.
Alternative approach?
I discussed with the author of issue #475 who agreed that the index is not important and that it probably has no correlation to URIs in the real world.
I think the better thing to do here is take his alternative suggestion and just change the README to state that odd numbered resources are text and even numbered resources are blobs. Currently it is backward.
Current odd numbered resources are text

Current event numbered resources are blobs

Current README says the opposite

Sorry to maybe cause more chaos by introducing a new idea (which might be wrong because I don't have a huge amount of context on this server), but maybe we avoid all this weirdness by just using intentionally random ids? E.g. just generate a uuid for everything, and either:
|
@domdomegg excellent suggestion. Removes any assumptions about array index being meaningful, axes the admittedly strange "even/odd" language from the description, and delivers the point that URIs can carry meaning. |
@claude can you give a shot at implementing the proposal from @domdomegg above? create it as a separate branch, if you can please. |
Claude finished @domdomegg's task —— View job Implementation Complete! ✅Successfully implemented UUID-based resources as proposed by @domdomegg. Changes Made
SummaryCreated a new branch Key Changes:
Benefits:
The implementation builds successfully and the server starts without errors. You can create a PR from the new branch: https://github.com/modelcontextprotocol/servers/pull/new/uuid-based-resources |
Fixes issue #475 where array indices (0-based) didn't match URI numbers (1-based), causing confusion.
Changes
test://static/resource/0
Resolves #475
Note: this is technically a breaking change, but since this is a demo server for testing, it seems like this shouldn't actually be an issue?
Generated with Claude Code