[OUAO-FRANK]

Description

Fixes #1156.

Cancelled legacy tasks currently return a successful tasks/result envelope containing only _meta. The cancellation transition closes done without storing an error, and the terminal-state guard then discards the handler's return value.

Record context.Canceled in that same terminal transition and return the existing REQUEST_INTERRUPTED code (-32800) during result retrieval. Waiting and repeated readers get the cancellation error; late handler completion cannot replace it. Ordinary failures keep INTERNAL_ERROR and successful results are unchanged.

This affects the supported legacy tasks/result path only. It does not change SEP-2663 tasks/get or the extension work in #987, nor the execution-lifetime fix in #1054.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature
• MCP spec compatibility implementation
• Breaking change
• Documentation update
• Code refactoring
• Performance improvement
• Tests only
• Other

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 (no new API or configuration)

Additional Information

Validated on Linux/WSL with Go 1.26.9; lint used the CI-pinned golangci-lint 2.8.0 with Go 1.25.5.

• Root race suite: all seven tested packages passed. The initial go test -mod=readonly -race -json -p 2 ./... -count=1 -timeout=600s exhausted its overall transport-package budget while compiling stdio fixtures; rerunning go test -mod=readonly -race -json -p 2 ./client/transport -count=1 -timeout=1800s passed in 434s. Other packages passed in the first run.
• Full ./server race suite rerun with all added cases: passed.
• cd otel && go test -mod=readonly -race -p 2 ./... -count=1 -timeout=180s: passed.
• go build -mod=readonly -p 2 ./..., go vet -mod=readonly -p 2 ./..., go generate ./..., and git diff --check: passed; generated source unchanged.
• Pinned golangci-lint run --timeout=5m in root and otel: 0 issues.

GOFLAGS=-buildvcs=false was inherited by child Go builds because this is a Windows-created worktree tested under WSL. No test or CI settings were changed.

The reproducer failed three times on unchanged main. Removing the stored cancellation error or restoring the wrong error classification via Go overlays makes the new assertions fail. Coverage includes cooperative regular-tool cancellation, task-tool cancellation/deadline, the non-terminal result waiting branch, repeated retrieval, late success, ordinary success/failure controls, and a real HTTP result envelope. The HTTP test internally creates/cancels the task; it is not a complete HTTP tools/call end-to-end test.

AI-assisted investigation, implementation and tests; local results were executed and the diff independently reviewed.

Summary by CodeRabbit

• Bug Fixes
  • Task results now report interrupted requests when a task is cancelled, instead of returning a generic internal error.
  • Cancelled tasks no longer accept a later successful completion, and cancellation state is preserved.
  • Subsequent tasks continue to operate normally after a task is cancelled.
  • Deadline-exceeded and other non-cancellation failures continue to return an internal error.

[mark-iii-labs-huly]

Connected to Huly®: MCP_G-716

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 36c1fdec-63d4-495e-9fa8-a659934ae388

Commits

Reviewing files that changed from the base of the PR and between c8b41fa and 9110bbb.

Files selected for processing: 2

• server/server.go
• server/task_cancel_result_test.go

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

Walkthrough

Task cancellation now stores context.Canceled as the task result error. Result retrieval maps that error to REQUEST_INTERRUPTED; other errors retain their existing classification.

Changes

Task cancellation result handling

Layer / File(s)	Summary
Record and return cancellation errors server/server.go, server/task_cancel_result_test.go	Task result handling returns REQUEST_INTERRUPTED for cancellation errors. Tests cover cancellation before and during result waits, repeated requests, late completion, task state, HTTP envelopes, and error classification.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Possibly related issues

• mark3labs/mcp-go#1156 — The PR changes cancelled task results from successful metadata-only responses to REQUEST_INTERRUPTED errors.

Suggested labels: type: bug, area: sdk

Suggested reviewers: jaormx

Merge Risk: Minimal · up to 9110b

Cancelled task results return an interruption error rather than a successful metadata-only response. No actionable merge-blocking risk remains in the supplied evidence.

Pre-merge checks: 3 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #1156 requires cancelled legacy tasks/result requests to return REQUEST_INTERRUPTED. The cancellation transition stores context.Canceled, and handleTaskResult maps that error correctly. …	Resolve the context.DeadlineExceeded classification mismatch. Update the result-error mapping if deadline errors must return REQUEST_INTERRUPTED, or change the added test if that behavior is not required. The complete added test suite m…
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title "fix(server): return errors for cancelled legacy task results" matches the main change. The raw summary says cancelling a task now stores context.Canceled as its result error, and task-resul…
Description check		The description covers the problem, the fix, and the scope. It explains that cancelled legacy tasks returned a successful metadata-only tasks/result envelope. It states that the fix records context.Ca…
Out of Scope Changes check		The changes are limited to the legacy task-result error path and regression tests for cancellation, waiting, repeated retrieval, late completion, HTTP envelopes, and error classification. No changes a…

Full details: Title check

Explanation

The title "fix(server): return errors for cancelled legacy task results" matches the main change. The raw summary says cancelling a task now stores context.Canceled as its result error, and task-result failures return REQUEST_INTERRUPTED when that error matches. The title is specific and concise, uses a conventional scope prefix, and states the bug fix from the developer's perspective.

Full details: Description check

Explanation

The description covers the problem, the fix, and the scope. It explains that cancelled legacy tasks returned a successful metadata-only tasks/result envelope. It states that the fix records context.Canceled in the terminal transition and returns REQUEST_INTERRUPTED (-32800). It notes that ordinary failures keep INTERNAL_ERROR and that SEP-2663 tasks/get is out of scope. It links issue #1156, marks Bug fix, and includes detailed test and validation notes. The template sections are all present. The Checklist item for documentation is left unchecked with an explanation. The 'Fixes #1156' line follows the template. The MCP Spec Compliance section was removed, which the template marks as optional. The description is complete and specific.

Full details: Out of Scope Changes check

Explanation

The changes are limited to the legacy task-result error path and regression tests for cancellation, waiting, repeated retrieval, late completion, HTTP envelopes, and error classification. No changes affect tasks/get, task lifetime, TTL, capacity, or the Tasks Extension.

Full details: Linked Issues check

Explanation

Issue #1156 requires cancelled legacy tasks/result requests to return REQUEST_INTERRUPTED. The cancellation transition stores context.Canceled, and handleTaskResult maps that error correctly. However, the added TestTaskResultErrorClassification expects REQUEST_INTERRUPTED for context.DeadlineExceeded. The implementation checks only errors.Is(resultErr, context.Canceled), so that case returns INTERNAL_ERROR and the added test fails.

Resolution

Resolve the context.DeadlineExceeded classification mismatch. Update the result-error mapping if deadline errors must return REQUEST_INTERRUPTED, or change the added test if that behavior is not required. The complete added test suite must pass.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts