diff --git a/actions/setup/js/claude_harness.cjs b/actions/setup/js/claude_harness.cjs index 52d08572a52..37935475322 100644 --- a/actions/setup/js/claude_harness.cjs +++ b/actions/setup/js/claude_harness.cjs @@ -45,7 +45,7 @@ const { fetchAWFReflect, fetchModelsFromUrl, } = require("./awf_reflect.cjs"); -const { emitMissingToolPermissionIssue, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); +const { emitMissingToolPermissionIssue, hasExpectedSafeOutputs, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); const { countPermissionDeniedIssues, hasNumerousPermissionDeniedIssues, extractDeniedCommands, buildMissingToolPermissionIssuePayload } = require("./permission_denied_helpers.cjs"); const { detectNonRetryableHarnessGuard } = require("./harness_retry_guard.cjs"); const { MODEL_NOT_SUPPORTED_PATTERN: INVALID_MODEL_ERROR_PATTERN } = require("./detect_agent_errors.cjs"); @@ -430,6 +430,14 @@ async function main() { } if (hasNumerousPermissionDenied) { + // If the agent already produced expected safe-outputs, the permission-denied + // signals are from optional/exploratory commands — not from the core task work. + // Suppress the terminal verdict and exit 0 to avoid a false-red run. + if (safeOutputsPath && hasExpectedSafeOutputs(safeOutputsPath, { logger: log })) { + log(`attempt ${attempt + 1}: detected numerous permission-denied issues but safe-outputs already contain expected output — suppressing terminal verdict (false-red: core work succeeded)`); + lastExitCode = 0; + break; + } const deniedCommands = extractDeniedCommands(result.output); emitMissingToolPermissionIssue({ deniedCommands, logger: log }); log(`attempt ${attempt + 1}: detected numerous permission-denied issues — not retrying (classified as missing tool/permission issue)`); @@ -521,6 +529,7 @@ if (typeof module !== "undefined" && module.exports) { buildMissingToolPermissionIssuePayload, emitMissingToolPermissionIssue, hasNoopInSafeOutputs, + hasExpectedSafeOutputs, }; } diff --git a/actions/setup/js/codex_harness.cjs b/actions/setup/js/codex_harness.cjs index 9f5706db8ce..3bd61944375 100644 --- a/actions/setup/js/codex_harness.cjs +++ b/actions/setup/js/codex_harness.cjs @@ -44,7 +44,7 @@ const { fetchAWFReflect, fetchModelsFromUrl, } = require("./awf_reflect.cjs"); -const { emitMissingToolPermissionIssue, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); +const { emitMissingToolPermissionIssue, hasExpectedSafeOutputs, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); const { countPermissionDeniedIssues, hasNumerousPermissionDeniedIssues, extractDeniedCommands, buildMissingToolPermissionIssuePayload } = require("./permission_denied_helpers.cjs"); const { detectNonRetryableHarnessGuard } = require("./harness_retry_guard.cjs"); const { MODEL_NOT_SUPPORTED_PATTERN: INVALID_MODEL_ERROR_PATTERN } = require("./detect_agent_errors.cjs"); @@ -492,6 +492,14 @@ async function main() { } if (hasNumerousPermissionDenied) { + // If the agent already produced expected safe-outputs, the permission-denied + // signals are from optional/exploratory commands — not from the core task work. + // Suppress the terminal verdict and exit 0 to avoid a false-red run. + if (safeOutputsPath && hasExpectedSafeOutputs(safeOutputsPath, { logger: log })) { + log(`attempt ${attempt + 1}: detected numerous permission-denied issues but safe-outputs already contain expected output — suppressing terminal verdict (false-red: core work succeeded)`); + lastExitCode = 0; + break; + } const deniedCommands = extractDeniedCommands(result.output); emitMissingToolPermissionIssue({ deniedCommands, logger: log }); log(`attempt ${attempt + 1}: detected numerous permission-denied issues — not retrying (classified as missing tool/permission issue)`); @@ -553,6 +561,7 @@ if (typeof module !== "undefined" && module.exports) { getConfiguredOpenAIPortFromReflect, validateCodexOpenAIBaseURLFromReflect, hasNoopInSafeOutputs, + hasExpectedSafeOutputs, }; } diff --git a/actions/setup/js/copilot_harness.cjs b/actions/setup/js/copilot_harness.cjs index ceb11de8192..5def87944e7 100644 --- a/actions/setup/js/copilot_harness.cjs +++ b/actions/setup/js/copilot_harness.cjs @@ -58,7 +58,7 @@ const { fetchModelsFromUrl, resolveCopilotSDKCustomProviderFromReflect, } = require("./awf_reflect.cjs"); -const { runSafeOutputsCLI, buildMissingToolAlternatives, emitMissingToolPermissionIssue, emitInfrastructureIncomplete, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); +const { runSafeOutputsCLI, buildMissingToolAlternatives, emitMissingToolPermissionIssue, emitInfrastructureIncomplete, hasExpectedSafeOutputs, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); const { countPermissionDeniedIssues, hasNumerousPermissionDeniedIssues, extractDeniedCommands, buildMissingToolPermissionIssuePayload } = require("./permission_denied_helpers.cjs"); const { detectNonRetryableHarnessGuard } = require("./harness_retry_guard.cjs"); const { isCAPIQuotaExceededError } = require("./detect_agent_errors.cjs"); @@ -892,6 +892,14 @@ async function main() { } if (hasNumerousPermissionDenied) { + // If the agent already produced expected safe-outputs, the permission-denied + // signals are from optional/exploratory commands — not from the core task work. + // Suppress the terminal verdict and exit 0 to avoid a false-red run. + if (safeOutputsPath && hasExpectedSafeOutputs(safeOutputsPath, { logger: log })) { + log(`attempt ${attempt + 1}: detected numerous permission-denied issues but safe-outputs already contain expected output — suppressing terminal verdict (false-red: core work succeeded)`); + lastExitCode = 0; + break; + } const deniedCommands = extractDeniedCommands(result.output); emitMissingToolPermissionIssue({ deniedCommands, logger: log }); log(`attempt ${attempt + 1}: detected numerous permission-denied issues — not retrying (classified as missing tool/permission issue)`); @@ -1035,6 +1043,7 @@ if (typeof module !== "undefined" && module.exports) { buildCopilotSDKServerArgs, getCopilotSDKServerPort, hasNoopInSafeOutputs, + hasExpectedSafeOutputs, isDetectionPhase, isModelAvailableInReflectData, isModelAvailableInReflectFile, diff --git a/actions/setup/js/copilot_harness.test.cjs b/actions/setup/js/copilot_harness.test.cjs index 747062783c4..726f3a7e7dc 100644 --- a/actions/setup/js/copilot_harness.test.cjs +++ b/actions/setup/js/copilot_harness.test.cjs @@ -27,6 +27,7 @@ const { extractDeniedCommands, hasNumerousPermissionDeniedIssues, hasNoopInSafeOutputs, + hasExpectedSafeOutputs, INFERENCE_ACCESS_ERROR_PATTERN, AGENTIC_ENGINE_TIMEOUT_PATTERN, isDetectionPhase, @@ -1906,4 +1907,69 @@ process.exit(1);`, expect(result.stderr).toContain("noop message found in safe-outputs — not retrying"); }); }); + + describe("permission-denied suppression when expected safe-outputs already produced", () => { + it("exits 0 and suppresses terminal verdict when numerous permission-denied occurs after expected safe-output was written", () => { + const tempDir = makeHarnessTempDir("copilot-perm-denied-suppression-"); + const safeOutputsPath = path.join(tempDir, "safe-outputs.jsonl"); + const stubPath = path.join(tempDir, "stub.cjs"); + const promptPath = path.join(tempDir, "prompt.txt"); + const callsPath = path.join(tempDir, "calls.jsonl"); + // Stub writes an expected safe-output then fails with numerous permission-denied output. + fs.writeFileSync( + stubPath, + `const fs = require("fs"); +const callsPath = process.env.COPILOT_HARNESS_STUB_CALLS; +const safeOutputsPath = process.env.GH_AW_SAFE_OUTPUTS; +fs.appendFileSync(callsPath, JSON.stringify({args: process.argv.slice(2)}) + "\\n"); +fs.appendFileSync(safeOutputsPath, JSON.stringify({type:"add_comment",body:"Report posted"}) + "\\n"); +process.stdout.write("permission denied\\npermission denied\\npermission denied\\nEACCES: permission denied\\nEPERM operation not permitted\\n"); +process.exit(1);`, + "utf8" + ); + fs.writeFileSync(promptPath, "fix the bug", "utf8"); + + const result = spawnSync(process.execPath, ["copilot_harness.cjs", process.execPath, stubPath, "--prompt-file", promptPath], { + cwd: path.dirname(require.resolve("./copilot_harness.cjs")), + env: { ...process.env, COPILOT_HARNESS_STUB_CALLS: callsPath, GH_AW_SAFE_OUTPUTS: safeOutputsPath }, + encoding: "utf8", + timeout: 15000, + }); + const callCount = fs.readFileSync(callsPath, "utf8").trim().split("\n").filter(Boolean).length; + // Only one attempt — no retries when permission-denied is suppressed + expect(callCount).toBe(1); + // Harness exits 0 because the core work (add_comment) already succeeded + expect(result.status).toBe(0); + expect(result.stderr).toContain("suppressing terminal verdict (false-red: core work succeeded)"); + }); + + it("exits 1 and emits missing_tool when numerous permission-denied occurs with no expected safe-outputs", () => { + const tempDir = makeHarnessTempDir("copilot-perm-denied-no-outputs-"); + const safeOutputsPath = path.join(tempDir, "safe-outputs.jsonl"); + const stubPath = path.join(tempDir, "stub.cjs"); + const promptPath = path.join(tempDir, "prompt.txt"); + const callsPath = path.join(tempDir, "calls.jsonl"); + // Stub fails with numerous permission-denied but writes no expected safe-output. + fs.writeFileSync( + stubPath, + `const fs = require("fs"); +const callsPath = process.env.COPILOT_HARNESS_STUB_CALLS; +fs.appendFileSync(callsPath, JSON.stringify({args: process.argv.slice(2)}) + "\\n"); +process.stdout.write("permission denied\\npermission denied\\npermission denied\\nEACCES: permission denied\\nEPERM operation not permitted\\n"); +process.exit(1);`, + "utf8" + ); + fs.writeFileSync(promptPath, "fix the bug", "utf8"); + + const result = spawnSync(process.execPath, ["copilot_harness.cjs", process.execPath, stubPath, "--prompt-file", promptPath], { + cwd: path.dirname(require.resolve("./copilot_harness.cjs")), + env: { ...process.env, COPILOT_HARNESS_STUB_CALLS: callsPath, GH_AW_SAFE_OUTPUTS: safeOutputsPath, GH_AW_SAFEOUTPUTS_CLI: "true" }, + encoding: "utf8", + timeout: 15000, + }); + // Harness exits 1 because no expected output was produced + expect(result.status).toBe(1); + expect(result.stderr).toContain("detected numerous permission-denied issues — not retrying"); + }); + }); }); diff --git a/actions/setup/js/safeoutputs_cli.cjs b/actions/setup/js/safeoutputs_cli.cjs index d0241640de6..25d9e72ebac 100644 --- a/actions/setup/js/safeoutputs_cli.cjs +++ b/actions/setup/js/safeoutputs_cli.cjs @@ -152,6 +152,58 @@ function emitInfrastructureIncomplete(details, options) { } } +/** + * Diagnostic safe-output types that represent infrastructure signals, not task-level work. + * Entries with these types are excluded when checking whether expected outputs were produced. + */ +const DIAGNOSTIC_SAFE_OUTPUT_TYPES = new Set(["noop", "missing_tool", "report_incomplete"]); + +/** + * Read the safe-outputs JSONL file and check whether it contains at least one + * non-diagnostic entry (i.e. an output that represents real, task-level work). + * Returns true when such an entry exists; false otherwise. + * Used by harnesses to suppress the terminal "numerous permission-denied" verdict + * when the agent already produced the expected output — preventing false-red runs. + * @param {string} safeOutputsPath - Path to the safe-outputs JSONL file + * @param {{ + * logger?: (msg: string) => void, + * readFileSync?: (path: string, encoding: BufferEncoding) => string + * }=} options + * @returns {boolean} + */ +function hasExpectedSafeOutputs(safeOutputsPath, options) { + const logger = options && options.logger ? options.logger : defaultLog; + const readFile = options && options.readFileSync ? options.readFileSync : fs.readFileSync; + + if (!safeOutputsPath) { + return false; + } + + let content; + try { + content = readFile(safeOutputsPath, "utf8"); + } catch { + // File does not exist or is not readable — no expected entries present + return false; + } + + const lines = content.split("\n"); + for (const line of lines) { + const trimmed = line.trim(); + if (!trimmed) continue; + try { + const parsed = JSON.parse(trimmed); + if (parsed && parsed.type && !DIAGNOSTIC_SAFE_OUTPUT_TYPES.has(parsed.type)) { + logger(`hasExpectedSafeOutputs: non-diagnostic entry found in ${safeOutputsPath}: type=${parsed.type}`); + return true; + } + } catch { + // Skip malformed lines — they do not represent valid entries + } + } + return false; +} + /** * Read the safe-outputs JSONL file and check whether any noop entry has been written. * Returns true when at least one {"type":"noop"} line is present; false otherwise. @@ -211,6 +263,7 @@ if (typeof module !== "undefined" && module.exports) { buildMissingToolAlternatives, emitMissingToolPermissionIssue, emitInfrastructureIncomplete, + hasExpectedSafeOutputs, hasNoopInSafeOutputs, }; } diff --git a/actions/setup/js/safeoutputs_cli.test.cjs b/actions/setup/js/safeoutputs_cli.test.cjs index 99de954da6c..56e8af76806 100644 --- a/actions/setup/js/safeoutputs_cli.test.cjs +++ b/actions/setup/js/safeoutputs_cli.test.cjs @@ -5,7 +5,7 @@ import os from "os"; import path from "path"; const require = createRequire(import.meta.url); -const { runSafeOutputsCLI, buildMissingToolAlternatives, emitMissingToolPermissionIssue, emitInfrastructureIncomplete, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); +const { runSafeOutputsCLI, buildMissingToolAlternatives, emitMissingToolPermissionIssue, emitInfrastructureIncomplete, hasExpectedSafeOutputs, hasNoopInSafeOutputs } = require("./safeoutputs_cli.cjs"); describe("safeoutputs_cli.cjs", () => { describe("runSafeOutputsCLI", () => { @@ -155,6 +155,138 @@ describe("safeoutputs_cli.cjs", () => { }); }); + describe("hasExpectedSafeOutputs", () => { + function makeTempFile(content) { + const p = path.join(os.tmpdir(), `safeoutputs-expected-test-${Date.now()}-${Math.random().toString(36).slice(2)}.jsonl`); + fs.writeFileSync(p, content, "utf8"); + return p; + } + + it("returns true when the file contains a non-diagnostic entry", () => { + const filePath = makeTempFile('{"type":"add_comment","body":"done"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(true); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns true for a submit_pull_request_review entry", () => { + const filePath = makeTempFile('{"type":"submit_pull_request_review","event":"APPROVE"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(true); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns false when all entries are diagnostic types", () => { + const filePath = makeTempFile('{"type":"noop","message":"nothing to do"}\n{"type":"missing_tool","tool":"x"}\n{"type":"report_incomplete","reason":"infra"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(false); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns false when the file contains only a noop entry", () => { + const filePath = makeTempFile('{"type":"noop","message":"nothing to do"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(false); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns false when the file contains only a missing_tool entry", () => { + const filePath = makeTempFile('{"type":"missing_tool","tool":"tool/permission"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(false); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns false when the file contains only a report_incomplete entry", () => { + const filePath = makeTempFile('{"type":"report_incomplete","reason":"infrastructure_error"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(false); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns true when an expected entry is mixed with diagnostic entries", () => { + const filePath = makeTempFile('{"type":"missing_tool","tool":"x"}\n{"type":"add_comment","body":"done"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(true); + } finally { + fs.rmSync(filePath); + } + }); + + it("returns false when the file does not exist", () => { + expect(hasExpectedSafeOutputs("/tmp/nonexistent-safe-outputs-expected.jsonl")).toBe(false); + }); + + it("returns false when safeOutputsPath is empty", () => { + expect(hasExpectedSafeOutputs("")).toBe(false); + }); + + it("returns false for an empty file", () => { + const filePath = makeTempFile(""); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(false); + } finally { + fs.rmSync(filePath); + } + }); + + it("skips malformed lines and returns false when no valid expected entry exists", () => { + const filePath = makeTempFile('not-json\n{"type":"noop"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(false); + } finally { + fs.rmSync(filePath); + } + }); + + it("skips malformed lines and returns true when a valid non-diagnostic entry follows them", () => { + const filePath = makeTempFile('not-json\n{"type":"add_comment","body":"done"}\n'); + try { + expect(hasExpectedSafeOutputs(filePath)).toBe(true); + } finally { + fs.rmSync(filePath); + } + }); + + it("uses injected readFileSync for testability", () => { + const logs = []; + const result = hasExpectedSafeOutputs("/fake/path.jsonl", { + readFileSync: () => '{"type":"update_pull_request","title":"fix"}\n', + logger: m => logs.push(m), + }); + expect(result).toBe(true); + expect(logs.some(m => m.includes("non-diagnostic entry found"))).toBe(true); + }); + + it("returns false when injected readFileSync returns only diagnostic entries", () => { + const result = hasExpectedSafeOutputs("/fake/path.jsonl", { + readFileSync: () => '{"type":"noop","message":"nothing"}\n', + }); + expect(result).toBe(false); + }); + + it("returns false when injected readFileSync throws", () => { + const result = hasExpectedSafeOutputs("/fake/path.jsonl", { + readFileSync: () => { + throw new Error("ENOENT"); + }, + }); + expect(result).toBe(false); + }); + }); + describe("hasNoopInSafeOutputs", () => { function makeTempFile(content) { const p = path.join(os.tmpdir(), `safeoutputs-noop-test-${Date.now()}-${Math.random().toString(36).slice(2)}.jsonl`);