Skip to content

Ensure all test client instances are cleaned up#3

Merged
stephentoub merged 1 commit into
mainfrom
fixtests
Mar 19, 2025
Merged

Ensure all test client instances are cleaned up#3
stephentoub merged 1 commit into
mainfrom
fixtests

Conversation

@stephentoub

Copy link
Copy Markdown
Contributor

No description provided.

@stephentoub stephentoub requested a review from halter73 March 19, 2025 21:22
@stephentoub stephentoub merged commit 555933a into main Mar 19, 2025
@stephentoub stephentoub deleted the fixtests branch March 19, 2025 21:27
PranavSenthilnathan added a commit to PranavSenthilnathan/csharp-sdk that referenced this pull request Jun 11, 2026
Three related fixes to the task-store wrapper in McpServerImpl, all in
the same catch chain around L920-970:

modelcontextprotocol#2 - Failed payload shape + JsonDocument lifetime (SEP-2663 186)
- Register JsonRpcErrorDetail in McpJsonUtilities source-gen context.
- Replace JsonDocument.Parse(...).RootElement (leaks pooled buffer)
  with typed JsonRpcErrorDetail + SerializeToElement.
- Emit {code, message} per spec; McpProtocolException preserves its
  ErrorCode + Message (documented safe to propagate); all other
  exceptions redact to InternalError + generic message.
- Test: McpProtocolException_FromTool_StoresAsFailedWithJsonRpcErrorShape.

modelcontextprotocol#3 - Orphan-on-IsTask=true
- When TaskStore is configured AND CallToolWithTaskHandler returns
  IsTask=true, the store's pre-created task was left in Working
  forever. Now fails the store's task with a clear InternalError
  identifying the misconfiguration (use only one mechanism).
- New test class: TaskStoreOrphanedTaskTests.

modelcontextprotocol#4 - Dedicated InputRequiredException catch
- When [McpServerTool] throws InputRequiredException under the task
  wrapper, it fell through to the generic catch and surfaced as a
  misleading Failed task. The taskId was already returned to the
  client synchronously, so InputRequiredResult cannot be surfaced
  retroactively. Now fails the task with an actionable InvalidRequest
  message pointing the user to CallToolWithTaskHandler.
- Test: InputRequiredException_FromTool_FailsTaskWithActionableMessage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant