Add lastModified field to Annotations for MCP spec 2025-11-25 #663
Conversation
WalkthroughAdd Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mcp/utils.go (1)
539-562: MissingLastModifiedparsing inParseAnnotations.The
ParseAnnotationsfunction handlespriorityandaudiencebut does not extract thelastModifiedfield from the input map. Per the PR objectives and theAnnotationsstruct definition,LastModifiedshould also be parsed here for consistency when deserializing annotation data.func ParseAnnotations(data map[string]any) *Annotations { if data == nil { return nil } annotations := &Annotations{} if value, ok := data["priority"]; ok { if value != nil { if priority, err := cast.ToFloat64E(value); err == nil { annotations.Priority = &priority } } } if value, ok := data["audience"]; ok { for _, a := range cast.ToStringSlice(value) { a := Role(a) if a == RoleUser || a == RoleAssistant { annotations.Audience = append(annotations.Audience, a) } } } + + if value, ok := data["lastModified"]; ok { + if str, ok := value.(string); ok { + annotations.LastModified = str + } + } return annotations }
🧹 Nitpick comments (3)
mcp/types.go (1)
968-983: Incomplete comment for Priority field.The comment on lines 978-980 appears truncated. It states "0 means 'least important,' and indicates that" but the sentence is incomplete. The new comment on line 979 partially overlaps with the original context.
Consider completing or restructuring the comment:
// Describes how important this data is for operating the server. // // A value of 1 means "most important," and indicates that the data is - // effectively required, while 0 means "least important," and indicates that - // Priority from 0.0 to 1.0 (1 = most important, 0 = least important) + // effectively required, while 0 means "least important," and indicates that + // the data is entirely optional. + // Priority ranges from 0.0 to 1.0 (1 = most important, 0 = least important). Priority *float64 `json:"priority,omitempty"`mcp/resources.go (1)
58-67: NewWithLastModifiedoption lacks timestamp validation.Similar to
WithAnnotations, this builder accepts any string without validating it's a valid ISO 8601 timestamp. WhileValidateISO8601Timestampexists, it's not used here. This could lead to invalid timestamps being stored silently.Either validate internally or document that callers must validate:
// WithLastModified adds a last modified timestamp to the Resource. // The timestamp should be in ISO 8601 format (e.g., "2025-01-12T15:00:58Z"). +// Callers should use ValidateISO8601Timestamp to validate the timestamp before use. func WithLastModified(timestamp string) ResourceOption { return func(r *Resource) { if r.Annotations == nil { r.Annotations = &Annotations{} } r.Annotations.LastModified = timestamp } }mcp/resources_test.go (1)
387-395: Test forWithLastModifiedverifies basic functionality.The test correctly validates that
Annotationsis created when nil and thatLastModifiedis properly assigned.Consider adding test cases for:
WithAnnotationswith a non-emptylastModifiedvalue- Combined usage of
WithAnnotationsandWithLastModifiedWithTemplateAnnotationswith a non-emptylastModifiedvaluefunc TestWithAnnotationsIncludingLastModified(t *testing.T) { resource := Resource{} timestamp := "2025-01-12T15:00:58Z" opt := WithAnnotations([]Role{RoleUser}, 1.0, timestamp) opt(&resource) require.NotNil(t, resource.Annotations) assert.Equal(t, timestamp, resource.Annotations.LastModified) assert.Equal(t, 1.0, *resource.Annotations.Priority) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
mcp/resources.go(3 hunks)mcp/resources_test.go(9 hunks)mcp/types.go(1 hunks)mcp/utils.go(1 hunks)mcp/utils_test.go(6 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
mcp/types.gomcp/utils_test.gomcp/resources.gomcp/utils.gomcp/resources_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Testing: use testify/assert and testify/require
Write table-driven tests using a tests := []struct{ name, ... } pattern
Go test files must end with _test.go
Files:
mcp/utils_test.gomcp/resources_test.go
🧠 Learnings (5)
📚 Learning: 2025-04-21T21:26:32.945Z
Learnt from: octo
Repo: mark3labs/mcp-go PR: 149
File: mcptest/mcptest.go:0-0
Timestamp: 2025-04-21T21:26:32.945Z
Learning: In the mcptest package, prefer returning errors from helper functions rather than calling t.Fatalf() directly, giving callers flexibility in how to handle errors.
Applied to files:
mcp/utils_test.go
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Testing: use testify/assert and testify/require
Applied to files:
mcp/utils_test.gomcp/resources_test.go
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Write table-driven tests using a tests := []struct{ name, ... } pattern
Applied to files:
mcp/utils_test.gomcp/resources_test.go
📚 Learning: 2025-06-20T20:39:51.870Z
Learnt from: lariel-fernandes
Repo: mark3labs/mcp-go PR: 428
File: www/docs/pages/servers/prompts.mdx:218-234
Timestamp: 2025-06-20T20:39:51.870Z
Learning: In the mcp-go library, the GetPromptParams.Arguments field is of type map[string]string, not map[string]interface{}, so direct string access without type assertions is safe and correct.
Applied to files:
mcp/utils_test.go
📚 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:
mcp/utils.go
🧬 Code graph analysis (2)
mcp/resources.go (2)
mcp/prompts.go (1)
Role(83-83)mcp/types.go (3)
Resource(715-734)Annotations(968-983)ResourceTemplate(743-764)
mcp/resources_test.go (3)
mcp/resources.go (4)
WithAnnotations(47-56)WithTemplateAnnotations(107-116)ValidateISO8601Timestamp(119-126)WithLastModified(60-67)mcp/types.go (4)
Resource(715-734)Annotated(988-990)Annotations(968-983)ResourceTemplate(743-764)mcp/tools.go (1)
Description(928-932)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (8)
mcp/utils_test.go (2)
4-8: LGTM!Import formatting is clean with standard library first, followed by third-party packages, per coding guidelines.
10-95: Test coverage is good but missinglastModifiedtest cases.The table-driven tests properly validate the pointer-based
Priorityhandling using theptr()helper (defined inresources_test.go). However, onceParseAnnotationsis updated to parselastModified, corresponding test cases should be added here.Consider adding test cases for
lastModifiedparsing after the implementation is updated:{ name: "lastModified only", data: map[string]any{ "lastModified": "2025-01-12T15:00:58Z", }, expected: &Annotations{ LastModified: "2025-01-12T15:00:58Z", }, }, { name: "all fields", data: map[string]any{ "priority": 1.0, "audience": []any{"user"}, "lastModified": "2025-01-12T15:00:58Z", }, expected: &Annotations{ Priority: ptr(1.0), Audience: []Role{"user"}, LastModified: "2025-01-12T15:00:58Z", }, },mcp/resources.go (3)
47-56: Breaking change:WithAnnotationssignature updated.The function now accepts a
lastModifiedparameter, which is a breaking change for existing callers. The implementation correctly storesPriorityas a pointer. However, thelastModifiedtimestamp is not validated before assignment.Consider whether to validate
lastModifiedwithin the builder to fail fast on invalid timestamps:func WithAnnotations(audience []Role, priority float64, lastModified string) ResourceOption { return func(r *Resource) { + // Note: Consider calling ValidateISO8601Timestamp(lastModified) here + // to catch invalid timestamps early, or document that validation + // is the caller's responsibility. if r.Annotations == nil { r.Annotations = &Annotations{} } r.Annotations.Audience = audience r.Annotations.Priority = &priority r.Annotations.LastModified = lastModified } }
107-116:WithTemplateAnnotationsmirrorsWithAnnotationscorrectly.The implementation is consistent with
WithAnnotations, correctly storingPriorityas a pointer and settingLastModified. Same consideration about timestamp validation applies here.
118-126:ValidateISO8601Timestampcorrectly uses RFC3339, which matches MCP spec requirements.The MCP specification requires ISO 8601 timestamps for the
lastModifiedfield in the format "2025-01-12T15:00:58Z" (RFC3339 with timezone), which the function correctly validates. The implementation appropriately allows empty strings for optional fields.mcp/resources_test.go (3)
317-317: LGTM!The
ptr()helper is a clean utility for creating float64 pointers in tests. This is a common Go testing pattern.
342-385: Thorough validation tests forValidateISO8601Timestamp.Good coverage of valid timestamps (Z and offset formats), empty string handling, and invalid formats. The table-driven approach follows coding guidelines.
45-78: Test cases updated for newWithAnnotationssignature.The tests correctly pass empty string
""forlastModifiedand useptr()for expectedPriorityvalues. The assertions properly compare pointer values.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
mcp/resources.go (1)
119-127:ValidateISO8601Timestampmatches the requested RFC3339‑based contractAllowing empty strings and using
time.Parse(time.RFC3339, timestamp)is exactly what the issue describes, and the helper returns the raw parse error for callers to wrap with context as needed. Consider very mildly tightening the docstring to mention RFC3339 explicitly to avoid implying full ISO‑8601 coverage, but the implementation itself is solid.mcp/resources_test.go (2)
46-317: Tests for pointer‑basedPriorityand annotation options are consistent, with a couple of minor polish opportunitiesAcross
TestNewResource,TestNewResourceTemplate,TestWithAnnotations,TestWithTemplateAnnotations, and the “creation from nil” tests, the updates to:
- pass the new
lastModifiedparameter (currently as""), and- assert
Priorityviaptr(...)or*...Annotations.Priorityaccurately reflect the new
*float64field and the updated option signatures. The smallptrhelper keeps expectations readable, and the tests verify that options correctly allocateAnnotationswhen starting from zero values.Two non‑blocking suggestions:
- The
Annotations.Prioritydoc comment inmcp/types.gosays the range is 0.0–1.0, but several tests still use values like2.0,2.5, or3.0. Consider normalizing these to the documented range (or adding validation in a future change) so examples don’t contradict the spec.TestResourceJSONMarshalingnow uses the newWithAnnotationssignature but still doesn’t assert onLastModified. If there isn’t JSON round‑trip coverage forlastModifiedelsewhere, it might be worth adding a variant here that sets a non‑empty timestamp and checks it survives marshal/unmarshal.Based on learnings, these tests follow the testify/table‑driven style the project prefers.
387-455:LastModifiedoption tests clearly define precedence semantics
TestWithLastModified,TestWithAnnotationsIncludingLastModified, andTestWithAnnotationsAndLastModifiedCombineddo a good job of specifying:
- that
WithLastModifiedallocatesAnnotationsas needed and setsLastModified, and- that whichever of
WithAnnotationsorWithLastModifiedruns last “wins” for the timestamp while preservingPriority.One tiny cleanup you might consider: in the first subtest of
TestWithAnnotationsAndLastModifiedCombined, theNewResource(...)call’s result is discarded and all assertions are done on a manually‑constructedresource. Either assert directly on theNewResourcereturn value or drop that call to avoid dead code in the test.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
mcp/resources.go(3 hunks)mcp/resources_test.go(9 hunks)mcp/types.go(1 hunks)mcp/utils.go(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- mcp/utils.go
- mcp/types.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
mcp/resources.gomcp/resources_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Testing: use testify/assert and testify/require
Write table-driven tests using a tests := []struct{ name, ... } pattern
Go test files must end with _test.go
Files:
mcp/resources_test.go
🧠 Learnings (2)
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Write table-driven tests using a tests := []struct{ name, ... } pattern
Applied to files:
mcp/resources_test.go
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Testing: use testify/assert and testify/require
Applied to files:
mcp/resources_test.go
🧬 Code graph analysis (2)
mcp/resources.go (1)
mcp/prompts.go (1)
Role(83-83)
mcp/resources_test.go (2)
mcp/resources.go (6)
WithAnnotations(47-56)WithTemplateAnnotations(108-117)NewResourceTemplate(77-88)ValidateISO8601Timestamp(120-127)WithLastModified(61-68)NewResource(16-27)mcp/types.go (4)
Resource(715-734)Annotated(989-991)Annotations(968-984)ResourceTemplate(743-764)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (5)
mcp/resources.go (4)
3-7: Import block and newtimedependency look correct
timeis only used byValidateISO8601Timestamp, and the import grouping (stdlib first, then third‑party) matches the Go style the repo targets. No issues here.
45-56:WithAnnotationscorrectly wiresPrioritypointer andLastModifiedThe option now ensures
Annotationsis non‑nil, setsAudience, and storesPriorityas a*float64via&priority, plus the newLastModifiedfield. This matches the updatedAnnotationsshape and MCP spec expectations, while keeping the existing “call the option to opt‑in” behavior.As per coding guidelines, the exported function comment starts with
WithAnnotationsand remains accurate enough after the signature change.
58-67:WithLastModifiedbehaves as a focused, non‑disruptive annotation optionThe helper allocates
Annotationswhen needed and only mutatesLastModified, preserving any existingAudience/Priority. That composes cleanly withWithAnnotations, allowing whichever option runs last to own the timestamp, as exercised in tests. Leaving ISO‑8601 validation to callers (guided by the comment) is a reasonable design given theResourceOptionsignature.
106-117:WithTemplateAnnotationsmirrors resource behavior correctlyTemplate annotations now receive
Audience,Priorityas*float64, andLastModifiedin the same way asWithAnnotationsfor resources. This keeps resource and template configuration symmetric and ensures templates also gainlastModifiedsupport automatically.mcp/resources_test.go (1)
342-385:ValidateISO8601Timestamptests cover the key success and failure modesThe table‑driven cases exercise:
- valid Z‑suffix timestamps,
- valid numeric‑offset timestamps,
- the special “empty string is allowed” path, and
- clearly invalid formats/values.
That aligns with the helper’s
time.RFC3339‑based behavior and the issue requirements.
…n in resource tests.
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @dask-58. * #663 (comment) The following files were modified: * `mcp/resources.go` * `mcp/utils.go`
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
mcp/resources_test.go (5)
46-78: Resource annotations tests correctly updated for pointer priority and new builder APIUsing
WithAnnotations(..., 0.5, "")/WithAnnotations(..., 1.0, "")and switching expectations toPriority: ptr(0.5)/ptr(1.0)keeps the tests aligned withPriority *float64and the newlastModifiedparameter. SincelastModifiedis passed as an empty string here, omitting it from the expected struct is correct because the zero value matches. You might optionally add an assertion thatAnnotations.LastModifiedis empty in one of these cases to document the default behavior, but that's not strictly necessary.
125-148: Template annotations tests correctly reflect pointer priority; optional LastModified assertionThe
WithTemplateAnnotationscalls now pass the extralastModifiedparameter as an empty string and the expectations dereference*template.Annotations.Priority, which matches the new type. Similar to the resource tests, you could add a quickassert.Empty(t, template.Annotations.LastModified)in one of these cases if you want to explicitly lock in the default behavior whenlastModifiedis not used, but the current coverage is sufficient.
175-207:TestWithAnnotationsupdated correctly to use new signature and pointer priorityUpdating the options to
WithAnnotations(tt.audience, tt.priority, "")and asserting against*resource.Annotations.Priorityis consistent withPriority *float64and ensures annotations are created when needed. If you want slightly broader coverage of the new field, you could add a case with a non-emptylastModifiedand assert bothPriorityandLastModifiedhere, but it's already well covered by later tests.
227-255:TestWithTemplateAnnotationscorrectly aligned with new APIThe template annotations tests now exercise the updated
WithTemplateAnnotations(audience, priority, lastModified)signature and verify the dereferenced pointer*template.Annotations.Priority. Similar to the resource-side tests, you might optionally assert thatLastModifiedis empty here to document that behavior for templates as well, but functionally this is solid.
262-278: JSON round‑trip test now verifies LastModified behaviorAdding
WithAnnotations(..., 1.0, "2025-01-01T12:00:00Z")and the subsequent assertions:
require.NotNil(t, unmarshaled.Annotations)- checking
resource.Annotations.LastModifiedequals the literal string- and then
resource.Annotations.LastModified == unmarshaled.Annotations.LastModifiedproperly validates that
lastModifiedis set by the builder and preserved across JSON marshal/unmarshal. If you want to simplify, you could collapse the last two lines into a singleassert.Equal(t, "2025-01-01T12:00:00Z", unmarshaled.Annotations.LastModified), but the current approach is clear and correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
mcp/resources_test.go(11 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
mcp/resources_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Testing: use testify/assert and testify/require
Write table-driven tests using a tests := []struct{ name, ... } pattern
Go test files must end with _test.go
Files:
mcp/resources_test.go
🧠 Learnings (2)
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Write table-driven tests using a tests := []struct{ name, ... } pattern
Applied to files:
mcp/resources_test.go
📚 Learning: 2025-10-13T09:35:20.180Z
Learnt from: CR
Repo: mark3labs/mcp-go PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-10-13T09:35:20.180Z
Learning: Applies to **/*_test.go : Testing: use testify/assert and testify/require
Applied to files:
mcp/resources_test.go
🧬 Code graph analysis (1)
mcp/resources_test.go (2)
mcp/resources.go (3)
WithAnnotations(47-56)ValidateISO8601Timestamp(120-127)WithLastModified(61-68)mcp/types.go (4)
Resource(715-734)Annotated(989-991)Annotations(968-984)ResourceTemplate(743-764)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (8)
mcp/resources_test.go (8)
298-307: Creation-from-nil test correctly covers pointer priority for resources
TestAnnotationsCreationFromNilnow usesWithAnnotations(..., 1.0, "")and asserts*resource.Annotations.Priority == 1.0, which directly guards the updated behavior that the option allocatesAnnotationswhen nil and sets the pointer-valuedPriority. This is a good regression test for the new struct shape.
309-318: Template creation-from-nil test correctly covers pointer priority
TestTemplateAnnotationsCreationFromNilmirrors the resource test forResourceTemplate, ensuring annotations are allocated andPriorityis set to0.5via pointer. This is consistent with theResourceTemplatestructure and the updated option semantics.
320-321: Helperptris a clean way to construct float64 pointers in testsThe
ptrhelper keeps expectations for pointer-valued priorities concise and readable across tests. This is idiomatic for tests and avoids cluttering assertions with inline pointer-taking.
345-388:TestValidateISO8601Timestampgives solid coverage of RFC3339/ISO8601 validationThe table-driven test covers:
- Valid Zulu timestamp
- Valid offset timestamp
- Empty string treated as valid
- Bad format and non-date strings treated as errors
This matches
ValidateISO8601Timestamp’s contract (RFC3339 parse with empty allowed). If you ever expand acceptable formats (e.g., fractional seconds), you can extend this table easily, but as-is it’s a good, concise suite. (Based on learnings, this also follows the preferred table-driven style.)
390-398:TestWithLastModifiedcorrectly verifies standalone LastModified optionThe test ensures
WithLastModifiedinitializesAnnotationswhen nil and setsLastModifiedwhile leaving other fields untouched, which is exactly the behavior implied by the option implementation. This is a useful focused test on the new helper.
400-409:TestWithAnnotationsIncludingLastModifiedvalidates combined priority and timestamp settingThis test confirms that
WithAnnotationssets bothPriorityandLastModifiedin one call, which aligns with the new signature. It nicely complementsTestWithLastModifiedto cover both ways of populating the timestamp.
411-441: Order-sensitive tests correctly capture overwrite semantics between optionsThe two subtests in
TestWithAnnotationsAndLastModifiedCombinedverify:
- When
WithAnnotations(..., ts1)is followed byWithLastModified(ts2)inNewResource, the later option overwritesLastModifiedbut preservesPriority.- When applying options manually to a
Resource,WithLastModified(ts1)followed byWithAnnotations(..., ts2)results inLastModified == ts2andPriority == 1.0.This is a clear and valuable check of option ordering semantics, which is easy to regress when modifying builder behavior.
443-452: Template annotations test properly exercises LastModified + priority on templates
TestWithTemplateAnnotationsIncludingLastModifiedensures thatWithTemplateAnnotationssets bothLastModifiedand pointer-valuedPriorityforResourceTemplate, mirroring the resource-side behavior. This helps guarantee parity between resources and templates with respect to the new field.
2 participants
Description
Implemented support for
lastModifiedfield inAnnotationsaccording to MCP specification version 2025-11-25.Key Changes:
Annotationsstruct inmcp/types.goto includeLastModified(ISO 8601 string) and changedPriorityto*float64for proper optional handling.WithAnnotationsandWithTemplateAnnotationsbuilder signatures inmcp/resources.goto acceptlastModified.WithLastModifiedbuilder method for convenience.ValidateISO8601Timestamphelper function.examples/everything/main.goto demonstrate usage.www/docs/pages/servers/resources.mdxto reflect these changes.Fixes #654
Type of Change
Checklist
MCP Spec Compliance
Additional Information
The change to
WithAnnotationsandWithTemplateAnnotationssignature is a breaking change for existing Go code. ThePriorityfield change inAnnotationsstruct is also a breaking change for direct struct initialization if using non-pointer values, but aligns better with JSON marshaling for optional fields.Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.