Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions actions/setup/js/mcp_server_core.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -772,9 +772,10 @@ async function handleRequest(server, request, defaultHandler) {
if (missing.length) {
const hasRequiredFields = tool.inputSchema && Array.isArray(tool.inputSchema.required) && tool.inputSchema.required.length > 0;
if (hasRequiredFields && Object.keys(args).length === 0) {
const schemaGuidance = generateEnhancedErrorMessage(tool.inputSchema.required, name, tool.inputSchema);

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.

[/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.

throw {
code: -32602,
message: `Empty arguments are not allowed — this tool is write-once, not a discovery probe. To inspect the schema, use the tools/list MCP method. To signal that no action is needed, call \`noop\` with a \`message\`.`,
message: `Empty arguments are not allowed — this tool is write-once, not a discovery probe. To inspect the schema, use the tools/list MCP method. To signal that no action is needed, call \`noop\` with a \`message\`.\n\n${schemaGuidance}`,
};
}
throw {
Expand Down Expand Up @@ -941,10 +942,11 @@ async function handleMessage(server, req, defaultHandler) {
if (missing.length) {
const hasRequiredFields = tool.inputSchema && Array.isArray(tool.inputSchema.required) && tool.inputSchema.required.length > 0;
if (hasRequiredFields && Object.keys(args).length === 0) {
const schemaGuidance = generateEnhancedErrorMessage(tool.inputSchema.required, name, tool.inputSchema);
server.replyError(
id,
-32602,
`Empty arguments are not allowed — this tool is write-once, not a discovery probe. To inspect the schema, use the tools/list MCP method. To signal that no action is needed, call \`noop\` with a \`message\`.`
`Empty arguments are not allowed — this tool is write-once, not a discovery probe. To inspect the schema, use the tools/list MCP method. To signal that no action is needed, call \`noop\` with a \`message\`.\n\n${schemaGuidance}`
);
return;
}
Expand Down
40 changes: 40 additions & 0 deletions actions/setup/js/mcp_server_core.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,9 @@ describe("mcp_server_core.cjs", () => {
expect(results[0].error.message).toContain("write-once, not a discovery probe");
expect(results[0].error.message).toContain("tools/list");
expect(results[0].error.message).toContain("noop");
// Schema guidance should be included so the model can retry without calling tools/list
expect(results[0].error.message).toContain("Example:");
expect(results[0].error.message).toContain("Required parameter");
Comment on lines +357 to +359

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.

[/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:");
});

});

it("should return enhanced error for partially-supplied but invalid required fields", async () => {
Expand Down Expand Up @@ -568,6 +571,43 @@ describe("mcp_server_core.cjs", () => {
expect(response.error.message).toContain("too short");
expect(response.error.message).toContain("20");
});

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

registerTool(server, {
name: "probe_tool",
description: "A tool to test probe detection",
inputSchema: {
type: "object",
properties: { input: { type: "string", description: "Some input" } },
required: ["input"],
},
handler: args => ({
content: [{ type: "text", text: `received: ${args.input}` }],
}),
});

const response = await handleRequest(server, {
jsonrpc: "2.0",
id: 10,
method: "tools/call",
params: {
name: "probe_tool",
arguments: {}, // completely empty — probe attempt
},
});

expect(response.error).toBeDefined();
expect(response.error.code).toBe(-32602);
expect(response.error.message).toContain("write-once, not a discovery probe");
expect(response.error.message).toContain("tools/list");
expect(response.error.message).toContain("noop");
// Schema guidance should be included so the model can retry without calling tools/list
expect(response.error.message).toContain("Example:");
expect(response.error.message).toContain("Required parameter");
});
});

describe("loadToolHandlers", () => {
Expand Down
Loading