diff --git a/actions/setup/js/mcp_cli_bridge.cjs b/actions/setup/js/mcp_cli_bridge.cjs index 269cd57e1f8..6c4fd98ea40 100644 --- a/actions/setup/js/mcp_cli_bridge.cjs +++ b/actions/setup/js/mcp_cli_bridge.cjs @@ -924,6 +924,22 @@ function showToolHelp(serverName, toolName, tools) { process.stdout.write(lines.join("\n") + "\n"); } +/** + * Determine whether the bridge should show tool help instead of invoking the tool + * with an empty arguments object. + * + * safeoutputs tools always require arguments, so any call with an empty argument + * object is a schema-discovery probe that would otherwise trigger a -32602 + * validation error and encourage wasteful retries. + * + * @param {string} serverName + * @param {Record} toolArgs + * @returns {boolean} + */ +function shouldShowToolHelpForEmptyArgs(serverName, toolArgs) { + return serverName === SAFEOUTPUTS_SERVER_NAME && Object.keys(toolArgs).length === 0; +} + /** * Collapse whitespace and trim long help text for compact output. * @@ -1109,9 +1125,10 @@ function isResultMessage(message) { * * @param {unknown} responseBody - Parsed JSON-RPC response body * @param {string} serverName - Server name (for logging) + * @param {string} [toolName] - Tool name, when known * @returns {Promise} */ -async function formatResponse(responseBody, serverName) { +async function formatResponse(responseBody, serverName, toolName = "") { const core = global.core; const messages = extractJSONRPCMessages(responseBody); renderProgressMessages(messages); @@ -1123,11 +1140,17 @@ async function formatResponse(responseBody, serverName) { const errRecord = resp.error; const message = "message" in errRecord ? String(errRecord.message || "Unknown error") : "Unknown error"; const code = "code" in errRecord && errRecord.code != null ? String(errRecord.code) : ""; + const isSafeOutputsEmptyArgs = serverName === SAFEOUTPUTS_SERVER_NAME && code === "-32602" && /Empty arguments are not allowed/i.test(message); + const hint = + isSafeOutputsEmptyArgs && toolName + ? `Hint: do not retry '${serverName} ${toolName}' with empty arguments. Run '${serverName} ${toolName} --help' to inspect the required options, or call 'noop' with a message if no action is needed.` + : ""; const errText = code ? `Error [${code}]: ${message}` : `Error: ${message}`; process.stderr.write(errText + "\n"); core.error(`[${serverName}] Tool call error: ${errText}`); auditLog(serverName, { event: "tool_error", error: errText }); process.exitCode = 1; + if (hint) process.stderr.write(hint + "\n"); return; } @@ -1231,6 +1254,13 @@ async function main() { const stdinContent = hasStdinJsonPayload(toolUserArgs) ? readStdinSync() : null; const { args: toolArgs, json: jsonOutput } = parseToolArgs(toolUserArgs, schemaProperties, stdinContent); + if (shouldShowToolHelpForEmptyArgs(serverName, toolArgs)) { + core.warning(`[${serverName}] No arguments provided for '${toolName}'; showing command help instead of calling the tool`); + auditLog(serverName, { event: "show_tool_help_empty_args", tool: toolName }); + showToolHelp(serverName, toolName, tools); + return; + } + core.info(`[${serverName}] Calling tool '${toolName}' with args: ${JSON.stringify(toolArgs)}${jsonOutput ? " (--json)" : ""}`); auditLog(serverName, { event: "call_start", tool: toolName, arguments: toolArgs }); @@ -1263,7 +1293,7 @@ async function main() { // --json: print the raw JSON-RPC response body await writeStdoutAndFlush(JSON.stringify(resp.body, null, 2) + "\n"); } else { - await formatResponse(resp.body, serverName); + await formatResponse(resp.body, serverName, toolName); } } catch (err) { const totalMs = Date.now() - callStartMs; @@ -1300,6 +1330,7 @@ module.exports = { writeStdoutAndFlush, showHelp, showToolHelp, + shouldShowToolHelpForEmptyArgs, hasStdinJsonPayload, readStdinSync, ensureSafeOutputsTools, diff --git a/actions/setup/js/mcp_cli_bridge.test.cjs b/actions/setup/js/mcp_cli_bridge.test.cjs index 739f2a470aa..e03ffe52b6b 100644 --- a/actions/setup/js/mcp_cli_bridge.test.cjs +++ b/actions/setup/js/mcp_cli_bridge.test.cjs @@ -3,7 +3,7 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { ensureSafeOutputsTools, formatResponse, hasStdinJsonPayload, parseToolArgs, readStdinSync, showHelp, showToolHelp, writeStdoutAndFlush } from "./mcp_cli_bridge.cjs"; +import { ensureSafeOutputsTools, formatResponse, hasStdinJsonPayload, parseToolArgs, readStdinSync, shouldShowToolHelpForEmptyArgs, showHelp, showToolHelp, writeStdoutAndFlush } from "./mcp_cli_bridge.cjs"; describe("mcp_cli_bridge.cjs", () => { let originalCore; @@ -190,6 +190,12 @@ describe("mcp_cli_bridge.cjs", () => { } }); + it("shows help instead of calling safeoutputs tools with an empty args object", () => { + expect(shouldShowToolHelpForEmptyArgs("safeoutputs", {})).toBe(true); + expect(shouldShowToolHelpForEmptyArgs("safeoutputs", { title: "Bug report" })).toBe(false); + expect(shouldShowToolHelpForEmptyArgs("other-server", {})).toBe(false); + }); + it("coerces scientific notation when schema properties are unavailable", () => { const { args } = parseToolArgs(["--max_tokens", "1e3", "--threshold", "-2E-4"], {}); @@ -248,6 +254,62 @@ describe("mcp_cli_bridge.cjs", () => { expect(process.exitCode).toBe(0); }); + it("adds a non-retry hint for safeoutputs empty-argument rejections", async () => { + await formatResponse( + { + error: { + code: -32602, + message: "Empty arguments are not allowed — this tool is write-once, not a discovery probe.", + }, + }, + "safeoutputs", + "create_issue" + ); + + const stderr = stderrChunks.join(""); + expect(stderr).toContain("Error [-32602]: Empty arguments are not allowed"); + expect(stderr).toContain("do not retry 'safeoutputs create_issue' with empty arguments"); + expect(stderr).toContain("safeoutputs create_issue --help"); + expect(process.exitCode).toBe(1); + }); + + it("omits non-retry hint when toolName is absent", async () => { + await formatResponse( + { + error: { + code: -32602, + message: "Empty arguments are not allowed — this tool is write-once, not a discovery probe.", + }, + }, + "safeoutputs" + // toolName omitted → defaults to "" + ); + + const stderr = stderrChunks.join(""); + expect(stderr).toContain("Error [-32602]"); + expect(stderr).not.toContain("do not retry"); + expect(process.exitCode).toBe(1); + }); + + it("does not add a non-retry hint for -32602 errors from non-safeoutputs servers", async () => { + await formatResponse( + { + error: { + code: -32602, + message: "Empty arguments are not allowed — this tool is write-once, not a discovery probe.", + }, + }, + "agenticworkflows", + "some_tool" + ); + + const stderr = stderrChunks.join(""); + expect(stderr).toContain("Error [-32602]"); + expect(stderr).not.toContain("do not retry"); + expect(stderr).not.toContain("--help"); + expect(process.exitCode).toBe(1); + }); + it("keeps top-level help compact for many commands", () => { const tools = Array.from({ length: 25 }, (_, i) => ({ name: `tool_${i + 1}`,