diff --git a/actions/setup/js/collect_ndjson_output.cjs b/actions/setup/js/collect_ndjson_output.cjs index b3615880f7f..0f01abf5d24 100644 --- a/actions/setup/js/collect_ndjson_output.cjs +++ b/actions/setup/js/collect_ndjson_output.cjs @@ -181,6 +181,22 @@ async function main() { return; } if (!fs.existsSync(outputFile)) { + // Before treating a missing outputs file as a graceful no-op, check whether + // the safeoutputs MCP gateway reported 0 registered tools during setup. + // When that flag exists the agent could not emit any safe outputs because + // every safeoutputs call failed with "unknown tool" — this is a gateway + // infrastructure failure, not an intentional no-op, and must surface as an + // error rather than a silent green run. + const runnerTemp = process.env.RUNNER_TEMP || "/home/runner/work/_temp"; + const gatewayEmptyFlagPath = `${runnerTemp}/gh-aw/safeoutputs/gateway_empty.flag`; + if (fs.existsSync(gatewayEmptyFlagPath)) { + core.setFailed( + `safeoutputs MCP gateway registered 0 tools during setup; the agent could not emit any safe outputs. ` + + `This is a gateway infrastructure failure, not a normal no-op. ` + + `Check the MCP gateway startup logs for ECONNRESET errors or delayed backend registration and re-run the workflow.` + ); + return; + } core.info(`Output file does not exist: ${outputFile} — no safe-output items were emitted; treating as empty collection (graceful no-op)`); const emptyOutput = { items: [], errors: [] }; const emptyOutputJson = JSON.stringify(emptyOutput); diff --git a/actions/setup/js/collect_ndjson_output.test.cjs b/actions/setup/js/collect_ndjson_output.test.cjs index 641a3681107..ab3762d4b7c 100644 --- a/actions/setup/js/collect_ndjson_output.test.cjs +++ b/actions/setup/js/collect_ndjson_output.test.cjs @@ -1,5 +1,6 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import fs from "fs"; +import os from "os"; import path from "path"; import { createRequire } from "module"; const _require = createRequire(import.meta.url); @@ -165,6 +166,32 @@ describe("collect_ndjson_output.cjs", () => { expect(mockCore.info).toHaveBeenCalledWith(`Output file does not exist: ${missingFile} — no safe-output items were emitted; treating as empty collection (graceful no-op)`), expect(mockCore.exportVariable).toHaveBeenCalledWith("GH_AW_AGENT_OUTPUT", path.join(TMP_GH_AW_PATH, AGENT_OUTPUT_FILENAME))); }), + it("should fail with infra error when safeoutputs gateway-empty flag exists and outputs file is missing", async () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "collect-gateway-empty-")); + const missingFile = path.join(tempDir, "nonexistent-outputs.jsonl"); + const flagDir = path.join(tempDir, "gh-aw", "safeoutputs"); + fs.mkdirSync(flagDir, { recursive: true }); + fs.writeFileSync(path.join(flagDir, "gateway_empty.flag"), ""); + const originalRunnerTemp = process.env.RUNNER_TEMP; + process.env.RUNNER_TEMP = tempDir; + process.env.GH_AW_SAFE_OUTPUTS = missingFile; + try { + await eval(`(async () => { ${collectScript}; await main(); })()`); + const failedCalls = mockCore.setFailed.mock.calls; + expect(failedCalls.length).toBeGreaterThan(0); + const failedMessage = failedCalls[0][0]; + expect(failedMessage).toContain("safeoutputs MCP gateway registered 0 tools"); + expect(failedMessage).toContain("gateway infrastructure failure"); + expect(mockCore.setOutput).not.toHaveBeenCalledWith("output", expect.anything()); + } finally { + if (originalRunnerTemp === undefined) { + delete process.env.RUNNER_TEMP; + } else { + process.env.RUNNER_TEMP = originalRunnerTemp; + } + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }), it("should error and still set output when artifact write fails", async () => { const writeError = new Error("disk full"); const spy = vi.spyOn(fs, "writeFileSync").mockImplementationOnce(() => { diff --git a/actions/setup/js/mount_mcp_as_cli.cjs b/actions/setup/js/mount_mcp_as_cli.cjs index f43d302c3d7..81164917104 100644 --- a/actions/setup/js/mount_mcp_as_cli.cjs +++ b/actions/setup/js/mount_mcp_as_cli.cjs @@ -61,8 +61,45 @@ function loadToolsFromJSONFile(toolsPath, core) { } /** - * Recover safeoutputs tools from the generated safe-outputs tools.json when MCP - * tools/list returned an empty result. + * Return the path where the safeoutputs gateway-empty flag file is written. + * The path is computed at call time (not module load time) so that tests can + * control the location by setting process.env.RUNNER_TEMP. + * + * @returns {string} + */ +function getSafeOutputsGatewayEmptyFlagPath() { + const runnerTemp = process.env.RUNNER_TEMP || "/home/runner/work/_temp"; + return path.join(runnerTemp, "gh-aw", "safeoutputs", "gateway_empty.flag"); +} + +/** + * Write a flag file that signals the safeoutputs MCP gateway registered 0 tools. + * collect_ndjson_output.cjs reads this flag and fails the conclusion job with a + * clear infra error instead of silently treating the missing outputs.jsonl as a + * graceful no-op. + * + * Failures are non-fatal — the flag is best-effort. A warning is emitted if the + * write fails so that the issue is still surfaced in the step log. + * + * @param {typeof import("@actions/core")} core + */ +function writeSafeOutputsGatewayEmptyFlag(core) { + const flagPath = getSafeOutputsGatewayEmptyFlagPath(); + try { + fs.mkdirSync(path.dirname(flagPath), { recursive: true }); + fs.writeFileSync(flagPath, "", { flag: "w" }); + } catch (err) { + core.warning(`Failed to write safeoutputs gateway-empty flag at ${flagPath}: ${getErrorMessage(err)}`); + } +} + +/** + * Validate the safeoutputs tool list and fail fast when the live gateway is empty. + * + * When the live gateway returns 0 tools, a flag file is written so that the + * conclusion job (collect_ndjson_output.cjs) can detect the outage and fail with + * a clear infra error instead of treating the missing outputs.jsonl as a graceful + * no-op. * * @param {Array<{name: string, description?: string, inputSchema?: unknown}>} tools * @param {typeof import("@actions/core")} core @@ -72,13 +109,13 @@ function recoverSafeOutputsToolsIfNeeded(tools, core) { if (tools.length > 0) { return tools; } - const fallbackPath = process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH || `${RUNNER_TEMP}/gh-aw/safeoutputs/tools.json`; - const recovered = loadToolsFromJSONFile(fallbackPath, core); - if (recovered.length > 0) { - core.warning(` safeoutputs tools/list returned empty; recovered ${recovered.length} tool(s) from ${fallbackPath}`); - return recovered; - } - throw new Error(`safeoutputs tool schema is empty (tools/list returned 0 and fallback ${fallbackPath} is empty/missing). ` + `Failing fast to avoid agent runs without discoverable safe-output tools.`); + + // The live MCP gateway returned 0 tools for safeoutputs. Write a flag file so + // that collect_ndjson_output.cjs can surface this as a hard failure instead of + // silently concluding "graceful no-op" when outputs.jsonl is never written. + writeSafeOutputsGatewayEmptyFlag(core); + + throw new Error(`safeoutputs tools/list returned 0 tools. ` + `Failing fast — the live MCP gateway has no tools registered. ` + `Check the MCP gateway startup logs for ECONNRESET errors or delayed backend registration.`); } /** @@ -552,6 +589,8 @@ module.exports = { toContainerUrl, loadToolsFromJSONFile, recoverSafeOutputsToolsIfNeeded, + getSafeOutputsGatewayEmptyFlagPath, + writeSafeOutputsGatewayEmptyFlag, SERVER_VALIDATORS, buildMCPCLIServersPromptList, }; diff --git a/actions/setup/js/mount_mcp_as_cli.test.cjs b/actions/setup/js/mount_mcp_as_cli.test.cjs index f2d1008ea39..d92eaf1d84e 100644 --- a/actions/setup/js/mount_mcp_as_cli.test.cjs +++ b/actions/setup/js/mount_mcp_as_cli.test.cjs @@ -4,7 +4,7 @@ import fs from "fs"; import os from "os"; import path from "path"; -import { AWF_GATEWAY_IP, buildMCPCLIServersPromptList, parseMCPResponseBody, recoverSafeOutputsToolsIfNeeded, toContainerUrl } from "./mount_mcp_as_cli.cjs"; +import { AWF_GATEWAY_IP, buildMCPCLIServersPromptList, getSafeOutputsGatewayEmptyFlagPath, parseMCPResponseBody, recoverSafeOutputsToolsIfNeeded, toContainerUrl } from "./mount_mcp_as_cli.cjs"; describe("mount_mcp_as_cli.cjs", () => { it("parses JSON object responses unchanged", () => { @@ -64,37 +64,80 @@ describe("mount_mcp_as_cli.cjs", () => { } }); - it("recovers empty safeoutputs tools from fallback tools.json", () => { + it("fails hard even when tools.json has tools, and writes gateway-empty flag", () => { const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mount-safeoutputs-")); const fallbackPath = path.join(tempDir, "tools.json"); fs.writeFileSync(fallbackPath, JSON.stringify([{ name: "create_issue" }]), "utf8"); const originalPath = process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; + const originalRunnerTemp = process.env.RUNNER_TEMP; + // Point at the tools.json to prove the function does not use it even when present process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH = fallbackPath; + process.env.RUNNER_TEMP = tempDir; try { - const warnings = []; - const recovered = recoverSafeOutputsToolsIfNeeded([], { warning: message => warnings.push(message) }); - expect(recovered).toHaveLength(1); - expect(recovered[0].name).toBe("create_issue"); - expect(warnings.join("\n")).toContain("recovered 1 tool(s)"); + // Must throw regardless of whether a fallback tools.json exists + expect(() => recoverSafeOutputsToolsIfNeeded([], { warning: () => {} })).toThrow(/safeoutputs tools\/list returned 0 tools/); + // Flag file must be written so collect_ndjson_output.cjs can detect the outage + const flagPath = getSafeOutputsGatewayEmptyFlagPath(); + expect(fs.existsSync(flagPath)).toBe(true); } finally { if (originalPath === undefined) { delete process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; } else { process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH = originalPath; } + if (originalRunnerTemp === undefined) { + delete process.env.RUNNER_TEMP; + } else { + process.env.RUNNER_TEMP = originalRunnerTemp; + } fs.rmSync(tempDir, { recursive: true, force: true }); } }); - it("throws when safeoutputs tools remain empty after fallback", () => { + it("throws when safeoutputs gateway returns 0 tools and writes gateway-empty flag", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mount-safeoutputs-empty-")); const originalPath = process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; - delete process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; + const originalRunnerTemp = process.env.RUNNER_TEMP; + process.env.RUNNER_TEMP = tempDir; + // Point at an explicitly missing file so there is no ambient fallback ambiguity + process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH = path.join(tempDir, "tools.json"); try { - expect(() => recoverSafeOutputsToolsIfNeeded([], { warning: () => {} })).toThrow(/safeoutputs tool schema is empty/); + expect(() => recoverSafeOutputsToolsIfNeeded([], { warning: () => {} })).toThrow(/safeoutputs tools\/list returned 0 tools/); + // Flag file must still be written even when the function throws + const flagPath = getSafeOutputsGatewayEmptyFlagPath(); + expect(fs.existsSync(flagPath)).toBe(true); } finally { - if (originalPath !== undefined) { + if (originalPath === undefined) { + delete process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; + } else { process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH = originalPath; } + if (originalRunnerTemp === undefined) { + delete process.env.RUNNER_TEMP; + } else { + process.env.RUNNER_TEMP = originalRunnerTemp; + } + fs.rmSync(tempDir, { recursive: true, force: true }); + } + }); + + it("does not write gateway-empty flag when tools list is non-empty", () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mount-safeoutputs-ok-")); + const originalRunnerTemp = process.env.RUNNER_TEMP; + process.env.RUNNER_TEMP = tempDir; + try { + const tools = [{ name: "push_to_pull_request_branch" }]; + const result = recoverSafeOutputsToolsIfNeeded(tools, { warning: () => {} }); + expect(result).toEqual(tools); + const flagPath = getSafeOutputsGatewayEmptyFlagPath(); + expect(fs.existsSync(flagPath)).toBe(false); + } finally { + if (originalRunnerTemp === undefined) { + delete process.env.RUNNER_TEMP; + } else { + process.env.RUNNER_TEMP = originalRunnerTemp; + } + fs.rmSync(tempDir, { recursive: true, force: true }); } });