fix: include schema guidance in empty-arguments error to prevent missing-tool reports

[Copilot]

When an agent (specifically o4-mini via AOAI/Entra) calls a safeoutputs tool with empty arguments {} as a schema probe, the server previously returned only the discovery-probe warning pointing to tools/list. The o4-mini model would not follow up with tools/list and instead emit a missing_tool safe-output — causing create_issue to be reported as unavailable in the Smoke Copilot - AOAI (Entra) run.

Changes

• mcp_server_core.cjs — Both empty-argument detection code paths now append generateEnhancedErrorMessage output (required fields + usage example) to the existing probe-detection error, giving the model immediate retry guidance without a tools/list roundtrip:

  Empty arguments are not allowed — this tool is write-once, not a discovery probe.
  To inspect the schema, use the tools/list MCP method. ...
  
  Invalid arguments: missing or empty 'title', 'body'
  
  Required parameter 'title': Concise issue title ...
  Required parameter 'body': Detailed issue description ...
  
  Example:
  {
    "title": "...",
    "body": "..."
  }
  

• mcp_server_core.test.cjs — Updated the probe-detection test to assert Required parameter and Example: are present in the error response.

[Copilot]

Pull request overview

This PR improves MCP tool-call validation errors when an agent sends empty {} arguments as a schema probe, by embedding schema guidance (required fields + example payload) directly in the probe-detection error so models can retry without a tools/list roundtrip.

Changes:

• Appends generateEnhancedErrorMessage(...) output to the existing “write-once, not a discovery probe” error for empty-arguments tool calls (in both request- and message-handling paths).
• Updates the probe-detection unit test to assert the presence of Required parameter guidance and an Example: block in the error message.

Show a summary per file

File	Description
actions/setup/js/mcp_server_core.cjs	Enhances empty-arguments probe-detection errors by appending schema guidance for immediate retry.
actions/setup/js/mcp_server_core.test.cjs	Extends the probe-detection test to verify schema guidance is included in the error message.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 2/2 changed files
• Comments generated: 1

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #40542 does not have the 'implementation' label (has_implementation_label=false) and has 0 new lines of code in business logic directories (requires_adr_by_default_volume=false, threshold=100). Neither Design Decision Gate condition is met.

[github-actions]

Skills-Based Review 🧠

Applied /diagnose and /tdd — approving with minor suggestions.

📋 Key Themes & Highlights

Key Themes

• Root cause well-addressed: The fix correctly identifies that o4-mini won't follow up on a tools/list redirect, and enriches the probe-detection error in-place with schema guidance so the model can retry immediately. Minimal blast radius.
• Symmetric patching: Both handleRequest (throws) and handleMessage (replyError) are updated consistently — no risk of one path regressing silently.
• Test gap: New assertions land only in the handleMessage describe block; the handleRequest path lacks a probe-detection test (see inline comment).
• Minor consistency: tool.inputSchema.required is passed to generateEnhancedErrorMessage in the empty-args branch while missing is used in the sibling partial-args branch — functionally equivalent but slightly inconsistent (see inline comment).

Positive Highlights

• ✅ Reuses the existing generateEnhancedErrorMessage helper — no logic duplication
• ✅ PR description clearly traces the regression from symptom → model behaviour → root cause → fix
• ✅ Test comment (// Schema guidance should be included so the model can retry without calling tools/list) serves as good inline documentation of the intent

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 66.3 AIC · ⌖ 11.5 AIC · ⊞ 6.9K

[github-actions]

[/tdd] These new assertions cover only the handleMessage probe-detection path. The handleRequest describe block (line 476) has no equivalent probe-detection test — a future regression there would go undetected.

💡 Suggested addition in the handleRequest describe block

it("should return probe-detection error with schema guidance for empty arguments", async () => {
  const { createServer, registerTool, handleRequest } = await import("./mcp_server_core.cjs");
  const server = createServer({ name: "safe-outputs", version: "1.0.0" });

  registerTool(server, {
    name: "create_issue",
    description: "Create an issue",
    inputSchema: {
      type: "object",
      properties: {
        title: { type: "string", description: "Issue title" },
        body:  { type: "string", description: "Issue body" },
      },
      required: ["title", "body"],
    },
    handler: () => ({ content: [] }),
  });

  const response = await handleRequest(server, {
    jsonrpc: "2.0", id: 10, method: "tools/call",
    params: { name: "create_issue", arguments: {} },
  });

  expect(response.error.code).toBe(-32602);
  expect(response.error.message).toContain("write-once, not a discovery probe");
  expect(response.error.message).toContain("Required parameter");
  expect(response.error.message).toContain("Example:");
});

[github-actions]

[/diagnose] Minor consistency nit: in the empty-args branch missing already equals tool.inputSchema.required (all fields are missing), so passing tool.inputSchema.required to generateEnhancedErrorMessage is equivalent but slightly diverges from the partial-args path just below (line 783) which uses missing.

💡 Suggested change

// Before
const schemaGuidance = generateEnhancedErrorMessage(tool.inputSchema.required, name, tool.inputSchema);

// After (matches the partial-args path)
const schemaGuidance = generateEnhancedErrorMessage(missing, name, tool.inputSchema);

Same change applies to line 945 in the handleMessage path. This makes both branches read consistently and avoids the impression that a different field list is intentional.

[github-actions]

No blocking issues found

The fix is correct and well-targeted. Both handleRequest and handleMessage correctly append generateEnhancedErrorMessage output to the probe-detection error, giving models like o4-mini immediate schema guidance without a tools/list roundtrip.

Analysis notes

• Correctness: tool.inputSchema.required is passed instead of the already-computed missing, but when Object.keys(args).length === 0, validateRequiredFields returns all required fields — they are guaranteed equal. No divergence.
• Error safety: generateEnhancedErrorMessage is fully guarded internally and is already called without try/catch in the surrounding non-probe paths — consistent with established pattern.
• Test update: Assertions for "Required parameter" and "Example:" correctly validate the new behavior on the handleMessage path.
• Missing handleRequest test: Already flagged in the prior review comment — not duplicated here.

No new issues found beyond what is already noted.

🔎 Code quality review by PR Code Quality Reviewer · 78.7 AIC · ⌖ 7.12 AIC · ⊞ 5.1K

[github-actions]

🧪 Test Quality Sentinel Report

✅ Test Quality Score: 100/100 — Excellent

Analyzed 1 test(s): 1 design, 0 implementation, 0 guideline violation(s).

📊 Metrics & Test Classification (1 test analyzed)

Metric	Value
New/modified tests analyzed	1
✅ Design tests (behavioral contracts)	1 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	1 (100%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	0

Test	File	Classification	Issues Detected
should return enhanced error for empty arguments	actions/setup/js/mcp_server_core.test.cjs:354	✅ Design	—

Go: 0; JavaScript: 1 (*.test.cjs). Other languages detected but not scored.

Test inflation check: test file +3 lines, production file +4 lines (ratio 0.75 — within 2:1 threshold).

Verdict

✅ Check passed. 0% implementation tests (threshold: 30%). The two new assertions verify the observable error-message contract: when a tool is called with empty arguments, the response must include inline schema guidance enabling the caller to self-correct without an additional tools/list round-trip.

🧪 Test quality analysis by Test Quality Sentinel · 67.5 AIC · ⊞ 8.3K · ◷

[github-actions]

✅ Test Quality Sentinel: 100/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%).

[pelikhan]

@copilot run pr-finisher skill

[Copilot]

@copilot run pr-finisher skill

Added a handleRequest-based probe-detection test in af41caf's follow-up commit. The new test (should return error for empty arguments object (probe detection) via handleRequest) registers a tool with a required field, calls handleRequest with empty {} arguments, and asserts the response contains write-once, not a discovery probe, tools/list, noop, Required parameter, and Example: — covering the HTTP/request-handler code path independently of the handleMessage path.