From aaeb7c60ea3abc416e32567c2d65f9df8124b3b1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:09:32 +0000 Subject: [PATCH 1/4] Initial plan From 7997b3dac6a2cc07d871e78ee4523d66f6643478 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:24:26 +0000 Subject: [PATCH 2/4] fix: detect safeoutputs gateway 0-tool outage and fail instead of silent no-op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the safeoutputs MCP gateway registers 0 tools during setup, the previous recovery path would log "recovered N tool(s) from tools.json" and continue, giving the false impression that safeoutputs was functional. In reality, the live gateway still served an empty tool list, so every MCP call from the agent failed with "unknown tool". Because outputs.jsonl was never written, collect_ndjson_output.cjs treated the run as a "graceful no-op" — success. This PR adds two changes to surface the outage: 1. mount_mcp_as_cli.cjs: when recoverSafeOutputsToolsIfNeeded detects the live gateway returned 0 tools (before or after file recovery), it writes a flag file at ${RUNNER_TEMP}/gh-aw/safeoutputs/gateway_empty.flag and updates the warning to clearly state the live gateway is broken. 2. collect_ndjson_output.cjs: before treating a missing outputs.jsonl as a graceful no-op, it checks for the gateway_empty.flag. When the flag is present, core.setFailed() is called with a clear infrastructure-failure message so the conclusion job fails visibly instead of reporting success. Fixes github/gh-aw#46184 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/collect_ndjson_output.cjs | 16 ++++++ .../setup/js/collect_ndjson_output.test.cjs | 27 ++++++++++ actions/setup/js/mount_mcp_as_cli.cjs | 52 ++++++++++++++++++- actions/setup/js/mount_mcp_as_cli.test.cjs | 50 ++++++++++++++++-- 4 files changed, 141 insertions(+), 4 deletions(-) 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..8f47e59e436 100644 --- a/actions/setup/js/mount_mcp_as_cli.cjs +++ b/actions/setup/js/mount_mcp_as_cli.cjs @@ -60,10 +60,48 @@ function loadToolsFromJSONFile(toolsPath, core) { } } +/** + * 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)}`); + } +} + /** * Recover safeoutputs tools from the generated safe-outputs tools.json when MCP * tools/list returned an empty result. * + * 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 * @returns {Array<{name: string, description?: string, inputSchema?: unknown}>} @@ -72,10 +110,20 @@ function recoverSafeOutputsToolsIfNeeded(tools, core) { if (tools.length > 0) { return 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); + 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}`); + core.warning( + `safeoutputs tools/list returned empty; recovered ${recovered.length} tool(s) from ${fallbackPath} for CLI help. ` + + `The live MCP gateway has 0 tools registered — MCP calls to safeoutputs will fail with "unknown tool". ` + + `Check the MCP gateway startup logs for ECONNRESET errors or delayed backend registration.` + ); 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.`); @@ -552,6 +600,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..b1e9c829178 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,81 @@ describe("mount_mcp_as_cli.cjs", () => { } }); - it("recovers empty safeoutputs tools from fallback tools.json", () => { + it("recovers empty safeoutputs tools from fallback tools.json 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; 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"); + // Warning must mention live gateway being broken, not just "recovered N tool(s)" expect(warnings.join("\n")).toContain("recovered 1 tool(s)"); + expect(warnings.join("\n")).toContain("live MCP gateway has 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 tools remain empty after fallback and still 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; + const originalRunnerTemp = process.env.RUNNER_TEMP; + process.env.RUNNER_TEMP = tempDir; delete process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; try { expect(() => recoverSafeOutputsToolsIfNeeded([], { warning: () => {} })).toThrow(/safeoutputs tool schema is empty/); + // Flag file must still be written even when the function throws + const flagPath = getSafeOutputsGatewayEmptyFlagPath(); + expect(fs.existsSync(flagPath)).toBe(true); } finally { if (originalPath !== undefined) { 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 }); } }); From 14f48b4618177565af1fc50913d6417bd9332127 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:52:24 +0000 Subject: [PATCH 3/4] Apply remaining changes Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/mount_mcp_as_cli.cjs | 19 ++++++------------ actions/setup/js/mount_mcp_as_cli.test.cjs | 23 +++++++++++----------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/actions/setup/js/mount_mcp_as_cli.cjs b/actions/setup/js/mount_mcp_as_cli.cjs index 8f47e59e436..067e4cd9a05 100644 --- a/actions/setup/js/mount_mcp_as_cli.cjs +++ b/actions/setup/js/mount_mcp_as_cli.cjs @@ -94,8 +94,7 @@ function writeSafeOutputsGatewayEmptyFlag(core) { } /** - * Recover safeoutputs tools from the generated safe-outputs tools.json when MCP - * tools/list returned an empty result. + * 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 @@ -116,17 +115,11 @@ function recoverSafeOutputsToolsIfNeeded(tools, core) { // silently concluding "graceful no-op" when outputs.jsonl is never written. writeSafeOutputsGatewayEmptyFlag(core); - 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} for CLI help. ` + - `The live MCP gateway has 0 tools registered — MCP calls to safeoutputs will fail with "unknown tool". ` + - `Check the MCP gateway startup logs for ECONNRESET errors or delayed backend registration.` - ); - 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.`); + 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.` + ); } /** diff --git a/actions/setup/js/mount_mcp_as_cli.test.cjs b/actions/setup/js/mount_mcp_as_cli.test.cjs index b1e9c829178..d92eaf1d84e 100644 --- a/actions/setup/js/mount_mcp_as_cli.test.cjs +++ b/actions/setup/js/mount_mcp_as_cli.test.cjs @@ -64,22 +64,18 @@ describe("mount_mcp_as_cli.cjs", () => { } }); - it("recovers empty safeoutputs tools from fallback tools.json and writes gateway-empty flag", () => { + 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"); - // Warning must mention live gateway being broken, not just "recovered N tool(s)" - expect(warnings.join("\n")).toContain("recovered 1 tool(s)"); - expect(warnings.join("\n")).toContain("live MCP gateway has 0 tools"); + // 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); @@ -98,19 +94,22 @@ describe("mount_mcp_as_cli.cjs", () => { } }); - it("throws when safeoutputs tools remain empty after fallback and still writes gateway-empty flag", () => { + 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; const originalRunnerTemp = process.env.RUNNER_TEMP; process.env.RUNNER_TEMP = tempDir; - delete process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; + // 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) { From 0af98dcfc46ca40f1b09e59f306dd2225c353ec6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 28 Jul 2026 13:57:46 +0000 Subject: [PATCH 4/4] fix: remove fallback-success path from recoverSafeOutputsToolsIfNeeded, fix test isolation Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- actions/setup/js/mount_mcp_as_cli.cjs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/actions/setup/js/mount_mcp_as_cli.cjs b/actions/setup/js/mount_mcp_as_cli.cjs index 067e4cd9a05..81164917104 100644 --- a/actions/setup/js/mount_mcp_as_cli.cjs +++ b/actions/setup/js/mount_mcp_as_cli.cjs @@ -115,11 +115,7 @@ function recoverSafeOutputsToolsIfNeeded(tools, core) { // 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.` - ); + 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.`); } /**