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
16 changes: 16 additions & 0 deletions actions/setup/js/collect_ndjson_output.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
27 changes: 27 additions & 0 deletions actions/setup/js/collect_ndjson_output.test.cjs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down Expand Up @@ -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(() => {
Expand Down
57 changes: 48 additions & 9 deletions actions/setup/js/mount_mcp_as_cli.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.`);
}

/**
Expand Down Expand Up @@ -552,6 +589,8 @@ module.exports = {
toContainerUrl,
loadToolsFromJSONFile,
recoverSafeOutputsToolsIfNeeded,
getSafeOutputsGatewayEmptyFlagPath,
writeSafeOutputsGatewayEmptyFlag,
SERVER_VALIDATORS,
buildMCPCLIServersPromptList,
};
65 changes: 54 additions & 11 deletions actions/setup/js/mount_mcp_as_cli.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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 });
}
});

Expand Down
Loading