Safe-outputs: add schema-driven synonym mapping (incl. camelCase aliases), keep synonym metadata internal to MCP/CLI prompts, and enforce strict unknown-parameter errors; clarify schema constraints#37421
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot merge main, populate synonym with likely LLM mistakes |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot make sure the synonyms are not communicated to the LLM through the MCP or cli generation. They are internal correction data, but should not pollute the prompts. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented in e7d29ba. |
There was a problem hiding this comment.
Pull request overview
This PR improves safe-outputs tool-call robustness by (1) normalizing tool arguments using schema-defined synonyms (including camelCase aliases), (2) stripping internal synonym metadata from MCP-exposed schemas, and (3) enforcing strict unknown-parameter rejection with “closest match” suggestions when schemas opt in via additionalProperties: false.
Changes:
- Add schema-driven synonym remapping (
x-synonyms) to safe-output argument normalization, and plumb the tool input schema into the normalizer (stdio + HTTP servers). - Sanitize tool schemas before exposing them via MCP (
tools/list) by strippingx-synonyms, while retaining original schemas internally for runtime normalization. - Add strict unknown-parameter validation in MCP core with nearest-valid-field suggestions and a bounded “supported parameters” preview; extend tests accordingly.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/js/safe_outputs_tools.json | Adds x-synonyms + doc clarifications to the safe-outputs tool schemas shipped with the workflow package. |
| actions/setup/js/safe_outputs_tools.json | Mirrors schema updates for the Actions setup copy (synonyms + doc clarifications). |
| actions/setup/js/safe_outputs_mcp_wrapped_arguments.test.cjs | Adds regression tests for synonym remapping and ensuring synonyms are not exposed via tools/list. |
| actions/setup/js/safe_outputs_mcp_server.cjs | Stores original schemas for normalization while stripping internal metadata from MCP-exposed schemas (stdio server). |
| actions/setup/js/safe_outputs_mcp_server_http.cjs | Same schema plumbing/sanitization as above for the HTTP server path. |
| actions/setup/js/safe_outputs_mcp_arguments.cjs | Implements synonym-based argument remapping and schema metadata stripping helper. |
| actions/setup/js/mcp_server_core.test.cjs | Adds coverage for strict unknown-parameter rejection + closest-parameter suggestions. |
| actions/setup/js/mcp_server_core.cjs | Implements unknown-parameter detection + error construction (gated by additionalProperties: false). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 2
| "message": { | ||
| "type": "string", | ||
| "description": "Status or completion message to log. Should explain what was analyzed and the outcome (e.g., 'Code review complete - no issues found', 'Analysis complete - all tests passing')." | ||
| "description": "Status or completion message to log. Should explain what was analyzed and the outcome (e.g., 'Code review complete - no issues found', 'Analysis complete - all tests passing'). This is the only content field noop accepts. Serialize any metrics or structured run summary into this single string; additional named fields (for example processed, skipped) are not accepted and cause a missing-message error." |
| "message": { | ||
| "type": "string", | ||
| "description": "Status or completion message to log. Should explain what was analyzed and the outcome (e.g., 'Code review complete - no issues found', 'Analysis complete - all tests passing')." | ||
| "description": "Status or completion message to log. Should explain what was analyzed and the outcome (e.g., 'Code review complete - no issues found', 'Analysis complete - all tests passing'). This is the only content field noop accepts. Serialize any metrics or structured run summary into this single string; additional named fields (for example processed, skipped) are not accepted and cause a missing-message error." |
Safe-output runs were incurring avoidable schema rejections due to ambiguous parameter naming (
levelvsseverity,commit_messagevsmessage), undocumented size constraints, andnooppayload shape confusion. This change adds schema-driven parameter synonym acceptance, keeps synonym metadata internal (not exposed to LLM-facing MCP/CLI schemas), and upgrades strict-schema validation to reject unknown parameters with nearest valid-field suggestions.Argument normalization: schema-defined synonyms
x-synonymsfrominputSchema.propertiesand remap aliases to canonical fields before required-field validation.issueNumber→issue_number).Synonym metadata kept internal (no prompt pollution)
x-synonyms) is stripped from MCP-exposed tool schemas (tools/list) and prompt-facing generation paths.Strict unknown-parameter handling
additionalProperties: false) in MCP core request handling.Safe-output schema/docs disambiguation and constraints
pkg/workflow/js/safe_outputs_tools.jsonandactions/setup/js/safe_outputs_tools.json) to:create_code_scanning_alert.severity(“NOT level”) and addx-synonyms: ["level"]push_to_pull_request_branch.message(“NOT commit_message”) and addx-synonyms: ["commit_message"]noop.messageas the only content field and require serializing structured metrics into that stringcreate_discussion.bodymax size (10 KB / 10240 bytes) with overflow guidancecategoryId/category_id,issue_number).Focused regression coverage
tools/listwhile synonym arguments are still accepted intools/call.