Skip to content

[safeoutputs] Clarify flat MCP arguments and add SafeOutputMCP wrapped-argument fallback#37208

Merged
pelikhan merged 9 commits into
mainfrom
copilot/safeoutputs-fix-create-discussion-params
Jun 5, 2026
Merged

[safeoutputs] Clarify flat MCP arguments and add SafeOutputMCP wrapped-argument fallback#37208
pelikhan merged 9 commits into
mainfrom
copilot/safeoutputs-fix-create-discussion-params

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jun 5, 2026

Agents were intermittently calling create_discussion with wrapped arguments ({"create_discussion": {...}}) instead of flat MCP tool arguments, causing first-attempt schema failures (missing or empty 'title', 'body') before retry recovery. The issue came from shared guidance that modeled wrapped NDJSON shape as if it were MCP call shape.

This change now addresses the problem in both places:

  • by correcting the guidance that teaches agents what to send, and

  • by making SafeOutputMCP resilient when agents still send wrapped arguments.

  • Tool schema guidance (create_discussion)

    • Updated create_discussion descriptions in both source-of-truth and runtime copies:
      • pkg/workflow/js/safe_outputs_tools.json
      • actions/setup/js/safe_outputs_tools.json
    • Added explicit instruction that arguments must be flat (title, body) and not nested under create_discussion.
  • Shared noop example correction

    • Updated .github/workflows/shared/noop-reminder.md to show flat MCP tool-call arguments and explicitly forbid wrapping under noop.
  • Runtime prompt propagation

    • Recompiled affected .lock.yml workflows so the corrected shared noop guidance is reflected in generated runtime prompts.
  • SafeOutputMCP wrapped-argument recovery

    • Added SafeOutputMCP-side argument normalization so that when a tool call arrives without a top-level type/flat argument shape, the server checks for a child object whose key matches the invoked tool name and uses that child object.
    • Wired this fallback into both safe-outputs MCP server entrypoints:
      • actions/setup/js/safe_outputs_mcp_server.cjs
      • actions/setup/js/safe_outputs_mcp_server_http.cjs
    • Added debug logging when SafeOutputMCP recovers wrapped arguments so fallback unwrapping can be confirmed from server logs.
    • Kept the fallback scoped to SafeOutputMCP rather than changing unrelated MCP servers.
  • Validation

    • Added focused coverage for wrapped safe-output MCP arguments, including the recovery logging path, and the shared normalization hook.

Example of corrected call shape:

{"message": "No action needed: [brief explanation]"}

(Previously shown as wrapped form: {"noop": {"message": "..."}}.)

Copilot AI and others added 3 commits June 5, 2026 22:13
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
…p guidance

Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix create_discussion parameters wrapping issue [safeoutputs] Clarify flat MCP argument shape for create_discussion and shared noop guidance Jun 5, 2026
Copilot AI requested a review from gh-aw-bot June 5, 2026 22:26
@pelikhan pelikhan marked this pull request as ready for review June 5, 2026 22:46
Copilot AI review requested due to automatic review settings June 5, 2026 22:46
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 5, 2026

@copilot if the SafeOutputMCP server cannot find the type field, it should check for a child that matches the tool name and use that child field. It should be resilient to these kinds of errors that agents tend to produce. This handling should be implemented at the SafeOutputMCP level.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Clarifies safe-output / MCP tool-call argument shapes to prevent intermittent schema failures caused by agents sending wrapped arguments (e.g., {"noop": {...}} / {"create_discussion": {...}}) instead of flat tool arguments.

Changes:

  • Updated create_discussion tool descriptions to explicitly require flat arguments (title, body) in both the source-of-truth and runtime tool schema JSON.
  • Corrected shared noop guidance to show flat MCP tool arguments and explicitly forbid wrapping under noop.
  • Regenerated selected .lock.yml workflows so the updated shared guidance is propagated into runtime prompts.
Show a summary per file
File Description
pkg/workflow/js/safe_outputs_tools.json Adds explicit flat-argument guidance to create_discussion tool description.
actions/setup/js/safe_outputs_tools.json Mirrors the create_discussion flat-argument guidance in the runtime copy.
.github/workflows/shared/noop-reminder.md Updates the shared noop example to flat arguments and forbids wrapping under noop.
.github/workflows/smoke-claude.lock.yml Regenerated prompt text to include the corrected noop guidance/example.
.github/workflows/daily-fact.lock.yml Regenerated prompt text (and related generated blocks) to include corrected noop guidance/example.
.github/workflows/daily-cache-strategy-analyzer.lock.yml Regenerated lock (metadata + generated block hashes) to propagate updated shared content.
.github/workflows/daily-agent-of-the-day-blog-writer.lock.yml Regenerated lock metadata to reflect updated shared content propagation.
.github/workflows/copilot-pr-nlp-analysis.lock.yml Regenerated lock metadata to reflect updated shared content propagation.

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: 3

Comment on lines +1 to 5
**Important**: If no action is needed after completing your analysis, you **MUST** call the `noop` safe-output tool with a brief explanation. Pass arguments directly to the tool. Do **NOT** wrap them under a `noop` key. Failing to call any safe-output tool is the most common cause of safe-output workflow failures.

```json
{"noop": {"message": "No action needed: [brief explanation of what was analyzed and why]"}}
{"message": "No action needed: [brief explanation of what was analyzed and why]"}
```
Comment on lines 101 to 104
{
"name": "create_discussion",
"description": "Create a GitHub discussion for announcements, Q&A, reports, status updates, or community conversations. Use this for content that benefits from threaded replies, doesn't require task tracking, or serves as documentation. For actionable work items that need assignment and status tracking, use create_issue instead.",
"description": "Create a GitHub discussion for announcements, Q&A, reports, status updates, or community conversations. Use this for content that benefits from threaded replies, doesn't require task tracking, or serves as documentation. For actionable work items that need assignment and status tracking, use create_issue instead. Arguments must be flat tool arguments (title, body), not nested under create_discussion.",
"inputSchema": {
Comment on lines 87 to 90
{
"name": "create_discussion",
"description": "Create a GitHub discussion for announcements, Q&A, reports, status updates, or community conversations. Use this for content that benefits from threaded replies, doesn't require task tracking, or serves as documentation. For actionable work items that need assignment and status tracking, use create_issue instead.",
"description": "Create a GitHub discussion for announcements, Q&A, reports, status updates, or community conversations. Use this for content that benefits from threaded replies, doesn't require task tracking, or serves as documentation. For actionable work items that need assignment and status tracking, use create_issue instead. Arguments must be flat tool arguments (title, body), not nested under create_discussion.",
"inputSchema": {
Copilot AI and others added 2 commits June 5, 2026 23:00
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot if the SafeOutputMCP server cannot find the type field, it should check for a child that matches the tool name and use that child field. It should be resilient to these kinds of errors that a...

Addressed in c3e5e04. SafeOutputMCP now unwraps a child object whose key matches the invoked tool name before required-field validation, and the safe-outputs stdio and HTTP servers both opt into that fallback.

Copilot AI changed the title [safeoutputs] Clarify flat MCP argument shape for create_discussion and shared noop guidance [safeoutputs] Clarify flat MCP arguments and add SafeOutputMCP wrapped-argument fallback Jun 5, 2026
Copilot AI requested a review from pelikhan June 5, 2026 23:02
@pelikhan
Copy link
Copy Markdown
Collaborator

pelikhan commented Jun 5, 2026

@copilot Add logging and unwrapping so we can confirm it occurred.

Copilot AI and others added 2 commits June 5, 2026 23:12
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Jun 5, 2026

@copilot Add logging and unwrapping so we can confirm it occurred.

Addressed in 71374b0. SafeOutputMCP now logs when it recovers wrapped arguments, including the tool key it unwrapped and the outer payload keys, and the recovery path is covered by focused tests.

@pelikhan pelikhan merged commit 9e6190b into main Jun 5, 2026
20 of 24 checks passed
@pelikhan pelikhan deleted the copilot/safeoutputs-fix-create-discussion-params branch June 5, 2026 23:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants