mcp: carry a cancelled notification's reason as the request's cancel cause - #1255
Open
jmrplens wants to merge 1 commit into
Open
mcp: carry a cancelled notification's reason as the request's cancel cause#1255jmrplens wants to merge 1 commit into
jmrplens wants to merge 1 commit into
Conversation
…cause The canceller read CancelledParams.Reason off the wire and dropped it: Connection.Cancel cancelled the request's context with a nil cause, so a handler that looked at context.Cause saw a bare context.Canceled and nothing of what the peer said. The specification asks implementations to log cancellation reasons, and the handler is the one place that can act on one. Connection.CancelCause is Cancel with a cause, and the canceller now cancels with an error that carries the peer's reason and unwraps to context.Canceled, so errors.Is keeps classifying the cancellation as one while context.Cause reads "request cancelled by the peer: <reason>". The canceller also logs the id and the reason at debug level through the logger the connection already carries. Two tests cover it: TestCancellation now asserts the cause the SDK's own client produces, and TestCancellationReason writes the call and the cancelled notification to the connection directly with a reason the client would never send. Fixes modelcontextprotocol#1254
jmrplens
force-pushed
the
cancel-reason
branch
from
September 12, 2026 19:46
6b5c161 to
2b50928
Compare
jmrplens
added a commit
to jmrplens/gitlab-mcp-server
that referenced
this pull request
Sep 12, 2026
…nd give six entries their summary rows (#739) A catch-up of `docs/development/upstream-bugs.md` against the upstream trackers as of today. Two more of the field-review merge requests to client-go landed on 2026-09-11 and the register still listed them as open: `!3053` (the four `Snippet` fields) shipped in v3.5.0 and `!3049` (`LastUsedAt` and `UsageType` on both deploy key structs) in v3.6.0, each version read from the tags that contain the merge commit rather than from the newest tag. The six that remain open all have a reviewer as of today, and `!3041` has its one review comment applied. On the documentation side, `!254519` is the second page merged into `master`, and the seven still open are in review since today; `!254542` has the technical writer's approval and waits on a pipeline whose only failures are the fork's `get_sources` step. Two defects of the register itself, found while reading it against the trackers: row 38 said the job token scope entry was neither reported nor in review, while its own entry has carried `gitlab-org/gitlab!254698` since 2026-09-10; and six entries that carry the five fields had no row in the summary table at all (the two project group listings, the ten modelled fields no entity exposes, `IssueRelation`, `MemberRole`, `PipelineInfo`, and the Group, Project and Issue pairs). The table is the index a reader scans first, so an entry it does not list is one the reader never reaches; they are rows 39 to 44 now, each read from its entry. The umbrella issue on client-go carries the same two merges in its table since today. The Go MCP SDK section moves too. The cancellation reason that the canceller discarded is now reported as modelcontextprotocol/go-sdk#1254 and fixed in modelcontextprotocol/go-sdk#1255, the first contribution to that SDK, chosen because the maintainers had already accepted the cause plumbing it builds on, it adds no exported API and it answers a SHOULD of the specification; the other SDK entries wait on its reception. Two entries were overtaken: the `Mcp-Name` decode was fixed upstream by another contributor in modelcontextprotocol/go-sdk#1242 (unreleased), and the malformed stdio frame was reported by another user in modelcontextprotocol/go-sdk#1209 with their own modelcontextprotocol/go-sdk#1210 open, so no second pull request goes there.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The canceller read
CancelledParams.Reasonoff the wire and dropped it:Connection.Cancelcancelled the request's context with a nil cause, so a handler that looked atcontext.Causesaw a barecontext.Canceledand nothing of what the peer said. The specification asks implementations to log cancellation reasons, and the handler is the one place that can act on one; #1100 had already given the request context a cause, so the reason has a natural place to travel.Connection.CancelCauseisCancelwith a cause. The canceller now cancels with an error that carries the peer's reason and unwraps tocontext.Canceled, soerrors.Iskeeps classifying the cancellation as one whilecontext.Causereadsrequest cancelled by the peer: <reason>. It also logs the id and the reason at debug level through the logger the connection already carries, which is the SDK's own answer to the SHOULD. No exported API changes: the cause type stays unexported and is read throughcontext.CauseandError().TestCancellationnow asserts the cause the SDK's own client produces (its context's error, which the client sends as the reason), andTestCancellationReasonwrites the call and the cancelled notification to the connection directly with a reason the client would never send, on a session pinned to 2025-11-25 so the hand-written request needs no_meta. Both fail without the change.Fixes #1254