diff --git a/actions/setup/js/add_labels.cjs b/actions/setup/js/add_labels.cjs index c160a8e77ba..65865b3141a 100644 --- a/actions/setup/js/add_labels.cjs +++ b/actions/setup/js/add_labels.cjs @@ -111,7 +111,11 @@ const main = createCountGatedHandler({ // In strict mode, objects without rationale and confidence are also rejected const missingMetadataLabels = requestedLabelInputs.filter(label => typeof label === "object" && label !== null && (!label.rationale || !label.confidence)); if (missingMetadataLabels.length > 0) { - const error = `Label objects must include both "rationale" and "confidence" when issue_intent is explicitly enabled. Missing metadata on: ${missingMetadataLabels.map(l => JSON.stringify(l.name)).join(", ")}`; + const missingMetadataLabelNames = missingMetadataLabels.map(l => { + const metadataLabel = /** @type {{name?: string}} */ l; + return JSON.stringify(metadataLabel.name ?? ""); + }); + const error = `Label objects must include both "rationale" and "confidence" when issue_intent is explicitly enabled. Missing metadata on: ${missingMetadataLabelNames.join(", ")}`; core.warning(error); return { success: false, error }; } diff --git a/actions/setup/js/copilot_harness.cjs b/actions/setup/js/copilot_harness.cjs index a220a0b770e..ff555713a3a 100644 --- a/actions/setup/js/copilot_harness.cjs +++ b/actions/setup/js/copilot_harness.cjs @@ -79,7 +79,16 @@ const PROMPT_FILE_INLINE_THRESHOLD_LABEL = "100KB"; const MAX_ENV_VAR_PREVIEW_LENGTH = 120; const OUTPUT_TAIL_MAX_CHARS = 600; const OUTPUT_TAIL_MAX_LINES = 12; -const POST_RESULT_WATCHDOG_IDLE_TIMEOUT_MS = 20 * 1000; +const MIN_POST_RESULT_WATCHDOG_TIMEOUT_MS = 50; +const DEFAULT_POST_RESULT_WATCHDOG_IDLE_TIMEOUT_MS = 20 * 1000; +function resolvePostResultWatchdogIdleTimeoutMs(env = process.env) { + const configuredTimeoutMs = Number(env.GH_AW_HARNESS_WATCHDOG_TIMEOUT_MS); + if (!Number.isFinite(configuredTimeoutMs) || configuredTimeoutMs <= 0) { + return DEFAULT_POST_RESULT_WATCHDOG_IDLE_TIMEOUT_MS; + } + return Math.max(MIN_POST_RESULT_WATCHDOG_TIMEOUT_MS, configuredTimeoutMs); +} +const POST_RESULT_WATCHDOG_IDLE_TIMEOUT_MS = resolvePostResultWatchdogIdleTimeoutMs(); const COPILOT_REQUESTS_PROXY_AUTH_403_TEMPLATE_NAME = "copilot_requests_proxy_auth_403.md"; // Pattern to detect transient CAPIError 400 in copilot output const CAPI_ERROR_400_PATTERN = /CAPIError:\s*400/; @@ -1170,6 +1179,20 @@ async function main() { break; } + // When the run fails with partial_execution and the safe-outputs file already contains a + // terminal result, treat the run as a success-with-late-activity rather than a + // retryable failure. This covers the common case where the post-result watchdog fires + // because the agent kept running exploratory commands after its deliverable was ready + // (watchdogFired=true), as well as any other partial_execution failure that occurs + // after the primary task output was already produced. Retrying would reproduce the + // same pattern and exhaust the retry budget without ever posting a final safe-output. + if (failureClass === "partial_execution" && safeOutputsPath && hasTerminalSafeOutput(safeOutputsPath)) { + const reason = result.watchdogFired ? "post-result watchdog fired after terminal safe-output was emitted" : "partial execution after terminal safe-output was already produced"; + log(`attempt ${attempt + 1}: ${reason} — treating as success (late-activity exit suppressed)`); + lastExitCode = 0; + break; + } + if (nonRetryableGuard.aiCreditsExceeded || nonRetryableGuard.awfAPIProxyBlockingRequests || isInvocationCapExceeded) { const reasons = []; if (nonRetryableGuard.aiCreditsExceeded) reasons.push("AI credits budget exceeded"); diff --git a/actions/setup/js/copilot_harness.test.cjs b/actions/setup/js/copilot_harness.test.cjs index 08dbf1880bd..4a89c7fe71c 100644 --- a/actions/setup/js/copilot_harness.test.cjs +++ b/actions/setup/js/copilot_harness.test.cjs @@ -2295,6 +2295,91 @@ process.exit(1);`, }); }); + describe("post-result watchdog suppression when terminal safe-output already produced", () => { + it("exits 0 without retrying when a terminal safe-output was already produced and run fails with partial_execution", () => { + const tempDir = makeHarnessTempDir("copilot-watchdog-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 a terminal safe-output, then remains alive until SIGTERM. + // This exercises the watchdog-fired partial_execution path end-to-end. + 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:"Daily report posted"}) + "\\n"); +process.stdout.write("Report uploaded. Checking logs...\\n"); +process.on("SIGTERM", () => process.exit(1)); +setInterval(() => {}, 1000);`, + "utf8" + ); + fs.writeFileSync(promptPath, "generate the report", "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_HARNESS_WATCHDOG_TIMEOUT_MS: "100", + }, + encoding: "utf8", + timeout: 15000, + }); + const callCount = fs.readFileSync(callsPath, "utf8").trim().split("\n").filter(Boolean).length; + // Only one attempt — no retries when watchdog suppression applies + expect(callCount).toBe(1); + // Harness exits 0 because the terminal safe-output was already produced + expect(result.status).toBe(0); + expect(result.stderr).toContain("post-result watchdog fired after terminal safe-output was emitted"); + expect(result.stderr).toContain("late-activity exit suppressed"); + }); + + it("still retries partial_execution when no terminal safe-output was produced (watchdog not armed)", () => { + const tempDir = makeHarnessTempDir("copilot-watchdog-no-output-"); + 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 produces output but exits 1 without writing any safe-output. + // The watchdog cannot fire (no terminal safe-output to arm it), so this + // should fall through to the normal partial-execution retry path. + 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("partial work done\\n"); +process.exit(1);`, + "utf8" + ); + fs.writeFileSync(promptPath, "generate the report", "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, + // Override retry config to keep the test fast. + GH_AW_HARNESS_MAX_RETRIES: "1", + GH_AW_HARNESS_INITIAL_DELAY_MS: "1", + }, + encoding: "utf8", + timeout: 15000, + }); + const callCount = fs.readFileSync(callsPath, "utf8").trim().split("\n").filter(Boolean).length; + // Should retry (2 attempts total with max_retries=1) + expect(callCount).toBeGreaterThan(1); + // Harness exits 1 because retries are exhausted with no output + expect(result.status).toBe(1); + expect(result.stderr).not.toContain("late-activity exit suppressed"); + }); + }); + describe("applyCopilotWireAPI", () => { afterEach(() => { delete process.env.COPILOT_MODEL; diff --git a/actions/setup/js/process_runner.cjs b/actions/setup/js/process_runner.cjs index 74db7a29301..771f1942412 100644 --- a/actions/setup/js/process_runner.cjs +++ b/actions/setup/js/process_runner.cjs @@ -69,7 +69,7 @@ function sleep(ms) { * - log - Caller-supplied logging function (harness-specific prefix) * - logArgs - Safe arg list used only for logging; defaults to `args`. * Pass a redacted copy to avoid leaking sensitive values. - * @returns {Promise<{exitCode: number, output: string, hasOutput: boolean, durationMs: number}>} + * @returns {Promise<{exitCode: number, output: string, hasOutput: boolean, durationMs: number, watchdogFired: boolean}>} */ function runProcess({ command, args, attempt, log, logArgs, env, postResultWatchdog }) { return new Promise(resolve => { @@ -78,7 +78,7 @@ function runProcess({ command, args, attempt, log, logArgs, env, postResultWatch // emits 'close' after 'error' (or vice-versa); only the first terminal event should // log and resolve so callers receive a deterministic result. let settled = false; - /** @param {{exitCode: number, output: string, hasOutput: boolean, durationMs: number}} result */ + /** @param {{exitCode: number, output: string, hasOutput: boolean, durationMs: number, watchdogFired: boolean}} result */ function settle(result) { if (settled) return; settled = true; @@ -172,8 +172,16 @@ function runProcess({ command, args, attempt, log, logArgs, env, postResultWatch child.on("close", (code, signal) => { const durationMs = Date.now() - startTime; const exitCode = code ?? 1; - log(`attempt ${attempt + 1}: process closed` + ` exitCode=${exitCode}` + (signal ? ` signal=${signal}` : "") + ` duration=${formatDuration(durationMs)}` + ` stdout=${stdoutBytes}B stderr=${stderrBytes}B hasOutput=${hasOutput}`); - settle({ exitCode, output: collectedOutput, hasOutput, durationMs }); + const watchdogFired = sentSigtermAt > 0; + log( + `attempt ${attempt + 1}: process closed` + + ` exitCode=${exitCode}` + + (signal ? ` signal=${signal}` : "") + + ` duration=${formatDuration(durationMs)}` + + ` stdout=${stdoutBytes}B stderr=${stderrBytes}B hasOutput=${hasOutput}` + + (watchdogFired ? ` watchdogFired=true` : "") + ); + settle({ exitCode, output: collectedOutput, hasOutput, durationMs, watchdogFired }); }); child.on("error", err => { @@ -188,6 +196,7 @@ function runProcess({ command, args, attempt, log, logArgs, env, postResultWatch output: collectedOutput, hasOutput, durationMs, + watchdogFired: false, }); }); }); diff --git a/actions/setup/js/process_runner.test.cjs b/actions/setup/js/process_runner.test.cjs index 95aef30d0c3..82401d9b0f0 100644 --- a/actions/setup/js/process_runner.test.cjs +++ b/actions/setup/js/process_runner.test.cjs @@ -201,6 +201,52 @@ describe("process_runner.cjs", () => { expect(logs.some(line => line.includes("post-result watchdog terminating idle process"))).toBe(true); }); + it("sets watchdogFired=true when post-result watchdog terminates the process", async () => { + const logs = []; + const result = await runProcess({ + command: process.execPath, + args: ["-e", 'process.stdout.write("done"); setInterval(() => {}, 1000);'], + attempt: 0, + log: msg => logs.push(msg), + postResultWatchdog: { + shouldArm: () => true, + inactivityTimeoutMs: 100, + pollIntervalMs: 25, + termGraceMs: 200, + }, + }); + expect(result.watchdogFired).toBe(true); + expect(logs.some(line => line.includes("watchdogFired=true"))).toBe(true); + }); + + it("sets watchdogFired=false when the process exits normally without the watchdog firing", async () => { + const logs = []; + const result = await runProcess({ + command: process.execPath, + args: ["-e", "process.exit(0)"], + attempt: 0, + log: msg => logs.push(msg), + }); + expect(result.watchdogFired).toBe(false); + }); + + it("sets watchdogFired=false when watchdog is configured but does not arm", async () => { + const logs = []; + const result = await runProcess({ + command: process.execPath, + args: ["-e", "setTimeout(() => process.exit(0), 100);"], + attempt: 0, + log: msg => logs.push(msg), + postResultWatchdog: { + shouldArm: () => false, + inactivityTimeoutMs: 50, + pollIntervalMs: 25, + termGraceMs: 100, + }, + }); + expect(result.watchdogFired).toBe(false); + }); + it("does not terminate processes when watchdog is not armed", async () => { const logs = []; const result = await runProcess({