Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
fix: strip and validate tool outputSchema and inputSchema by DaleSeo · Pull Request #860 · modelcontextprotocol/rust-sdk · GitHub
Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix: strip and validate tool outputSchema and inputSchema by DaleSeo · Pull Request #860 · modelcontextprotocol/rust-sdk · GitHub
Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix: strip and validate tool outputSchema and inputSchema by DaleSeo · Pull Request #860 · modelcontextprotocol/rust-sdk · GitHub
Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' fix: strip and validate tool outputSchema and inputSchema by DaleSeo · Pull Request #860 · modelcontextprotocol/rust-sdk · GitHub
Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' fix: strip and validate tool outputSchema and inputSchema by DaleSeo · Pull Request #860 · modelcontextprotocol/rust-sdk · GitHub
Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix: strip and validate tool outputSchema and inputSchema by DaleSeo · Pull Request #860 · modelcontextprotocol/rust-sdk · GitHub
Skip to content

fix: strip and validate tool outputSchema and inputSchema - #860

Merged
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description
Jun 2, 2026
Merged

fix: strip and validate tool outputSchema and inputSchema#860
alexhancock merged 2 commits into
mainfrom
fix/output-schema-strip-title-description

Conversation

@DaleSeo

@DaleSeoDaleSeo commented May 28, 2026

Copy link
Copy Markdown
Member

Motivation and Context

PR #856 cleaned up inputSchema by stripping the unnecessary top-level title and description fields, since the MCP spec does not require them and they only add noise to the LLM context. The same noise applies to outputSchema, and the spec's official examples for both schemas omit those top-level fields in identical fashion. This PR extends the same cleanup to outputSchema for symmetry.

image

EDIT: While auditing the symmetry, I found two related gaps and also addressed them. First, schema_for_input didn't enforce the MCP spec's type: "object" constraint, which schema_for_output already does. This meant that corner cases like Parameters<String> could silently create tool definitions that violated the spec. Now, the function is validated and returns Result<Arc<JsonObject>, String>, matching schema_for_output, with clear panics at call sites that previously couldn't fail. Second, schema_for_empty_input uses LazyLock to avoid allocating memory with each call.

How Has This Been Tested?

Added some unit tests and updated the integration test.

Breaking Changes

No breaking change. schema_for_input was added as a public function in PR #856 but has not been released yet.

Behavior change worth noting: tools whose input parameter type does not produce a type: "object" schema (e.g. Parameters<String>, Parameters<i32>) will now panic at construction time with a descriptive message rather than silently producing a spec-violating tool definition. Two tools inside this repo's own test suite fit that pattern and have been updated.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Documentation
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have added or updated documentation as needed

Additional context

@DaleSeoDaleSeo self-assigned this May 28, 2026
@github-actionsgithub-actionsBot added T-documentation Documentation improvements T-test Testing related changes T-config Configuration file changes T-core Core library changes T-handler Handler implementation changes T-macros Macro changes T-model Model/data structure changes labels May 28, 2026
@DaleSeo
DaleSeo marked this pull request as ready for review May 28, 2026 21:41
@DaleSeo
DaleSeo requested a review from a team as a code ownerMay 28, 2026 21:41
@jokemanfire
jokemanfire requested a review from CopilotMay 29, 2026 02:15

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR tightens and harmonizes MCP tool schema generation by (1) stripping top-level title/description from both inputSchema and outputSchema, and (2) validating that generated tool schemas have a root JSON Schema type: "object" (as required by the MCP spec), failing fast with clearer panics at tool-construction call sites.

Changes:

  • Strip top-level title/description from tool outputSchema (matching the existing inputSchema cleanup) and add shared validation/stripping logic.
  • Make schema_for_input validate root type: "object" and return Result, updating call sites to unwrap with descriptive panic messages.
  • Update tests/fixtures to use structured request/response types (object schemas) and validate outputSchema serialization behavior.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
FileDescription
README.mdUpdates documentation around schema generation (currently needs clarification; see PR comment).
crates/rmcp/src/handler/server/common.rsAdds shared validate+strip helper; enforces object-root schemas for tool input/output; caches results; optimizes empty-input schema via LazyLock.
crates/rmcp/src/model/tool.rsUpdates with_input_schema to unwrap Result with a clearer panic (matching output behavior).
crates/rmcp/src/handler/server/router/tool/tool_traits.rsUpdates default input_schema() to unwrap Result with a clearer panic.
crates/rmcp/src/handler/server/router/tool.rsUpdates router builder paths to unwrap schema_for_input with clearer panics.
crates/rmcp-macros/src/tool.rsUpdates macro-generated tool metadata to unwrap schema_for_input failures with descriptive panics.
crates/rmcp/tests/test_structured_output.rsAdjusts test tools to use object-typed parameter structs to comply with the new input-schema validation.
crates/rmcp/tests/test_list_tools_result.rsChanges tool to return Json<AddResult> so an outputSchema is generated and test can validate it.
crates/rmcp/tests/test_list_tools_result/list_tools_result.jsonUpdates fixture to include outputSchema (without top-level title/description).

Comment threadREADME.md
```

The generated tool `inputSchema` is derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
The generated tool `inputSchema` and `outputSchema` are derived from the fields of `T`. The type name and documentation on `T` are ignored; only field names, field types, and field documentation are used.
@alexhancock
alexhancock merged commit 254f04a into mainJun 2, 2026
19 checks passed
@alexhancock
alexhancock deleted the fix/output-schema-strip-title-description branch June 2, 2026 16:01
@github-actionsgithub-actionsBot mentioned this pull request Jun 2, 2026
jrmelsha added a commit to UserGeneratedLLC/rmcp-rust-sdk that referenced this pull request Jun 15, 2026
Brings in upstream v1.6.1..266f870 (15 commits): protocol-version
negotiation/init-mismatch fixes, input/outputSchema strip+validate
(modelcontextprotocol#856/modelcontextprotocol#860), tool-argument errors as tool results (modelcontextprotocol#894), OAuth issuer
validation + OIDC application_type + offline_access reauth, roots/
sampling/logging deprecation (SEP-2577), empty-2xx notification handling.
Conflict resolutions (fork changes preserved):
- handler/server/common.rs: kept both test blocks. Upstream's
validate_and_strip / schema_for_input layer composes on top of the
fork's ReplaceBoolSchemas normalization in schema_for_type.
- handler/server/router/tool.rs: combined upstream's
into_tool_argument_error dispatch with the anthropic-ext
normalize_call_tool_result post-processing (result kept mutable).
Restored .githooks/commit-msg, which upstream modelcontextprotocol#898 deleted incidentally
in an unrelated "progress demo" change; the hook gates our conventional-
commit workflow and skips merge commits.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

T-configConfiguration file changesT-coreCore library changesT-documentationDocumentation improvementsT-handlerHandler implementation changesT-macrosMacro changesT-modelModel/data structure changesT-testTesting related changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@DaleSeo@alexhancock