mcp: change CompleteParams.Context from pointer to value type

[SamMorrowDrums]

mcp: change CompleteParams.Context from pointer to value type

The CompleteContext struct contains only a single field (Arguments map[string]string)
which already handles the nil/empty case gracefully. Making Context a pointer type
forces all consumers to perform nil checks before accessing Arguments, which is
error-prone and has caused nil pointer panics in production (see github/github-mcp-server#1754).

With a value type, consumers can safely access req.Params.Context.Arguments directly:

• If the client omits the field, Arguments will be nil (safe to range over, check length, etc.)
• If the client sends an empty object, Arguments will be an empty map

This follows the Go idiom of preferring value types for simple structs where
the zero value is useful, similar to how time.Time is typically used as a value.

Before (requires nil check):

var resolved map[string]string
if req.Params.Context != nil {
    resolved = req.Params.Context.Arguments
}

After (safe direct access):

resolved := req.Params.Context.Arguments

[jba]

1. That would be a breaking change.
2. Making it a pointer future-proofs against future additions to struct, by avoiding the case where a large struct gets copied.

Sorry about the extra null check.

[SamMorrowDrums]

@jba I'd have been inclined to agree except that omitempty handles empty json object {} as a value meaning that while it's not totally unreasonable to have a double guard, it's also possibly leaking the client implementation a bit as to whether they send the empty object or not.

Perhaps we could remove the omitempty and leave it as a pointer?

[jba]

omitempty handles empty json object {} as a value

Do you mean:

• nil will marshal by omitting the field
• An empty struct will marshal with {} as the field value?

I'm not sure I see the problem. Don't all our struct pointers have the same issue?

[SamMorrowDrums]

@jba I'm very much ok to drop this, but tried to clarify my thinking below for posterity.

I was only thinking it's a little bit strange that the SDK internal representation would be temperamental to varying client implementations of parts the spec itself, so when I develop with one client I might get different structs than another. User input is different of course. One could argue that's a schema validation issue and if the schema proscribed one or another way, then it could be handled as a validation error to be fair.

I think I'd be more comfortable with json being marshalled so that an empty object is nil than occasionally being an empty struct depending on input.

We take full responsibility for not handling a potentially nil struct properly, that part I'm not attributing to the SDK at all. My reason to keep responding was only a philosophical one. I want the SDK spec fields to be deterministic, so regardless of client I get consistent go structs from the SDK.

But, again it's a pointer so always handle nil is fair answer.