From f43cf337ba3814ee3ce3448fdac0e93bf3d62fd8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 12:12:02 +0000 Subject: [PATCH 1/3] Initial plan From df945f1eef03113ff07b3d8ed98947e301c9bf59 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 12:31:08 +0000 Subject: [PATCH 2/3] fix: allow add_comment through when code push fails, prepend failure note Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5918ff1a-172d-40b8-b11f-1f2a54d79d32 --- ...ment-not-cancelled-on-code-push-failure.md | 5 ++ .../setup/js/safe_output_handler_manager.cjs | 15 +++- .../js/safe_output_handler_manager.test.cjs | 83 ++++++++++++++++--- 3 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 .changeset/patch-add-comment-not-cancelled-on-code-push-failure.md diff --git a/.changeset/patch-add-comment-not-cancelled-on-code-push-failure.md b/.changeset/patch-add-comment-not-cancelled-on-code-push-failure.md new file mode 100644 index 00000000000..06ca34f4401 --- /dev/null +++ b/.changeset/patch-add-comment-not-cancelled-on-code-push-failure.md @@ -0,0 +1,5 @@ +--- +"gh-aw": patch +--- + +Allow `add_comment` safe outputs to be posted even when a preceding code push operation (`create_pull_request` or `push_to_pull_request_branch`) fails. The comment body is annotated with a warning note describing the failure so users see the outcome. Previously, the status comment was silently cancelled when patch application failed, leaving no update in the issue or PR. diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 683a22c5df4..7cf8fa0662d 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -363,8 +363,10 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) continue; } - // Fail-fast: if a previous code-push operation failed, cancel non-code-push messages - if (codePushFailures.length > 0 && !CODE_PUSH_TYPES.has(messageType)) { + // Fail-fast: if a previous code-push operation failed, cancel non-code-push messages. + // Exception: add_comment messages are allowed through so the status comment still reaches + // the user — they will be annotated with a failure note (see effectiveMessage logic below). + if (codePushFailures.length > 0 && !CODE_PUSH_TYPES.has(messageType) && messageType !== "add_comment") { const cancelReason = `Cancelled: code push operation failed (${codePushFailures[0].type}: ${codePushFailures[0].error})`; core.info(`⏭ Message ${i + 1} (${messageType}) cancelled — ${cancelReason}`); results.push({ @@ -459,6 +461,15 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) effectiveMessage = { ...message, body: fallbackNote + (message.body || "") }; core.info(`Prepending fallback correction note to add_comment body (fallback issue: #${codePushFallbackInfo.issueNumber})`); } + // If a previous code-push operation failed outright (e.g. patch application error), + // prepend a failure warning to add_comment bodies so the status comment accurately + // reflects that the code changes were not applied. + if (messageType === "add_comment" && codePushFailures.length > 0) { + const failure = codePushFailures[0]; + const failureNote = `\n\n---\n> [!WARNING]\n> The \`${failure.type}\` operation failed: ${failure.error}. The code changes were not applied.\n\n`; + effectiveMessage = { ...effectiveMessage, body: failureNote + (effectiveMessage.body || "") }; + core.info(`Prepending code push failure note to add_comment body (${failure.type}: ${failure.error})`); + } // Call the message handler with the individual message and resolved temp IDs const result = await messageHandler(effectiveMessage, resolvedTemporaryIds, temporaryIdMap); diff --git a/actions/setup/js/safe_output_handler_manager.test.cjs b/actions/setup/js/safe_output_handler_manager.test.cjs index d7219d2d8a4..ebbedf691bb 100644 --- a/actions/setup/js/safe_output_handler_manager.test.cjs +++ b/actions/setup/js/safe_output_handler_manager.test.cjs @@ -964,11 +964,11 @@ describe("Safe Output Handler Manager", () => { }); describe("code-push fail-fast behaviour", () => { - it("should cancel subsequent messages when push_to_pull_request_branch fails", async () => { + it("should cancel subsequent non-add_comment messages when push_to_pull_request_branch fails", async () => { const messages = [{ type: "push_to_pull_request_branch" }, { type: "add_comment", body: "Success!" }, { type: "create_issue", title: "Issue" }]; const codePushHandler = vi.fn().mockResolvedValue({ success: false, error: "Branch not found" }); - const commentHandler = vi.fn(); + const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]); const issueHandler = vi.fn(); const handlers = new Map([ @@ -987,22 +987,24 @@ describe("Safe Output Handler Manager", () => { // First result: code-push failed expect(result.results[0].success).toBe(false); expect(result.results[0].error).toBe("Branch not found"); - // Subsequent results: cancelled - expect(result.results[1].success).toBe(false); - expect(result.results[1].cancelled).toBe(true); - expect(result.results[1].reason).toContain("Cancelled"); + // add_comment is NOT cancelled — it should be called with a failure note prepended + expect(result.results[1].cancelled).toBeUndefined(); + expect(commentHandler).toHaveBeenCalledTimes(1); + const calledMessage = commentHandler.mock.calls[0][0]; + expect(calledMessage.body).toContain("push_to_pull_request_branch"); + expect(calledMessage.body).toContain("Branch not found"); + // create_issue IS still cancelled (non-add_comment non-code-push type) expect(result.results[2].success).toBe(false); expect(result.results[2].cancelled).toBe(true); - // Subsequent handlers were NOT called - expect(commentHandler).not.toHaveBeenCalled(); + // create_issue handler was NOT called expect(issueHandler).not.toHaveBeenCalled(); }); - it("should cancel subsequent messages when create_pull_request fails via exception", async () => { + it("should allow add_comment through when create_pull_request fails via exception", async () => { const messages = [{ type: "create_pull_request" }, { type: "add_comment", body: "PR created!" }]; const codePushHandler = vi.fn().mockRejectedValue(new Error("API error")); - const commentHandler = vi.fn(); + const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]); const handlers = new Map([ ["create_pull_request", codePushHandler], @@ -1015,8 +1017,13 @@ describe("Safe Output Handler Manager", () => { expect(result.codePushFailures).toHaveLength(1); expect(result.codePushFailures[0].type).toBe("create_pull_request"); expect(result.codePushFailures[0].error).toBe("API error"); - expect(result.results[1].cancelled).toBe(true); - expect(commentHandler).not.toHaveBeenCalled(); + // add_comment is NOT cancelled — handler is called with failure note + expect(result.results[1].cancelled).toBeUndefined(); + expect(commentHandler).toHaveBeenCalledTimes(1); + const calledMessage = commentHandler.mock.calls[0][0]; + expect(calledMessage.body).toContain("create_pull_request"); + expect(calledMessage.body).toContain("API error"); + expect(calledMessage.body).toContain("PR created!"); }); it("should NOT cancel subsequent code-push messages after a code-push failure", async () => { @@ -1213,6 +1220,58 @@ describe("Safe Output Handler Manager", () => { expect(calledMessage.body).toBe("A fix PR has been created."); expect(calledMessage.body).not.toContain("pull request was not created"); }); + + it("should prepend failure note to add_comment body when create_pull_request fails (e.g. patch application error)", async () => { + const messages = [ + { type: "create_pull_request", title: "My Fix PR" }, + { type: "add_comment", body: "The agent has completed its work." }, + ]; + + const prHandler = vi.fn().mockResolvedValue({ success: false, error: "Failed to apply patch" }); + const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]); + + const handlers = new Map([ + ["create_pull_request", prHandler], + ["add_comment", commentHandler], + ]); + + await processMessages(handlers, messages); + + // add_comment handler must have been called (not cancelled) + expect(commentHandler).toHaveBeenCalledTimes(1); + const calledMessage = commentHandler.mock.calls[0][0]; + // Failure note should be prepended + expect(calledMessage.body).toContain("create_pull_request"); + expect(calledMessage.body).toContain("Failed to apply patch"); + expect(calledMessage.body).toContain("The agent has completed its work."); + // Note should appear before the original body + const noteIndex = calledMessage.body.indexOf("create_pull_request"); + const bodyIndex = calledMessage.body.indexOf("The agent has completed its work."); + expect(noteIndex).toBeLessThan(bodyIndex); + }); + + it("should prepend failure note to add_comment body when push_to_pull_request_branch fails", async () => { + const messages = [ + { type: "push_to_pull_request_branch", branch: "fix-branch" }, + { type: "add_comment", body: "Changes have been pushed." }, + ]; + + const pushHandler = vi.fn().mockResolvedValue({ success: false, error: "Branch not found" }); + const commentHandler = vi.fn().mockResolvedValue([{ _tracking: null }]); + + const handlers = new Map([ + ["push_to_pull_request_branch", pushHandler], + ["add_comment", commentHandler], + ]); + + await processMessages(handlers, messages); + + expect(commentHandler).toHaveBeenCalledTimes(1); + const calledMessage = commentHandler.mock.calls[0][0]; + expect(calledMessage.body).toContain("push_to_pull_request_branch"); + expect(calledMessage.body).toContain("Branch not found"); + expect(calledMessage.body).toContain("Changes have been pushed."); + }); }); describe("output emission via emitSafeOutputActionOutputs", () => { From cb2be64ef85adddb54f045603a9f55394e49eb1d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 25 Mar 2026 12:32:48 +0000 Subject: [PATCH 3/3] refactor: consolidate add_comment note-prepending into single guard block Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5918ff1a-172d-40b8-b11f-1f2a54d79d32 --- .../setup/js/safe_output_handler_manager.cjs | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 7cf8fa0662d..54b0c612220 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -452,23 +452,26 @@ async function processMessages(messageHandlers, messages, onItemCreated = null) // Record the temp ID map size before processing to detect new IDs const tempIdMapSizeBefore = temporaryIdMap.size; - // If a previous code-push operation fell back to a review issue, prepend a correction - // note to add_comment bodies so the posted comment accurately reflects the outcome. - // The note is placed before the AI-generated body so users see the clarification immediately. + // For add_comment messages: prepend any relevant correction notes before the AI-generated + // body so users see the clarification immediately. let effectiveMessage = message; - if (messageType === "add_comment" && codePushFallbackInfo) { - const fallbackNote = `\n\n---\n> [!NOTE]\n> The pull request was not created — a fallback review issue was created instead due to protected file changes: [#${codePushFallbackInfo.issueNumber}](${codePushFallbackInfo.issueUrl})\n\n`; - effectiveMessage = { ...message, body: fallbackNote + (message.body || "") }; - core.info(`Prepending fallback correction note to add_comment body (fallback issue: #${codePushFallbackInfo.issueNumber})`); - } - // If a previous code-push operation failed outright (e.g. patch application error), - // prepend a failure warning to add_comment bodies so the status comment accurately - // reflects that the code changes were not applied. - if (messageType === "add_comment" && codePushFailures.length > 0) { - const failure = codePushFailures[0]; - const failureNote = `\n\n---\n> [!WARNING]\n> The \`${failure.type}\` operation failed: ${failure.error}. The code changes were not applied.\n\n`; - effectiveMessage = { ...effectiveMessage, body: failureNote + (effectiveMessage.body || "") }; - core.info(`Prepending code push failure note to add_comment body (${failure.type}: ${failure.error})`); + if (messageType === "add_comment") { + // If a previous code-push operation fell back to a review issue, prepend a correction note + // so the posted comment accurately reflects the outcome. + if (codePushFallbackInfo) { + const fallbackNote = `\n\n---\n> [!NOTE]\n> The pull request was not created — a fallback review issue was created instead due to protected file changes: [#${codePushFallbackInfo.issueNumber}](${codePushFallbackInfo.issueUrl})\n\n`; + effectiveMessage = { ...effectiveMessage, body: fallbackNote + (effectiveMessage.body || "") }; + core.info(`Prepending fallback correction note to add_comment body (fallback issue: #${codePushFallbackInfo.issueNumber})`); + } + // If a previous code-push operation failed outright (e.g. patch application error), + // prepend a failure warning so the status comment accurately reflects that the + // code changes were not applied. + if (codePushFailures.length > 0) { + const failure = codePushFailures[0]; + const failureNote = `\n\n---\n> [!WARNING]\n> The \`${failure.type}\` operation failed: ${failure.error}. The code changes were not applied.\n\n`; + effectiveMessage = { ...effectiveMessage, body: failureNote + (effectiveMessage.body || "") }; + core.info(`Prepending code push failure note to add_comment body (${failure.type}: ${failure.error})`); + } } // Call the message handler with the individual message and resolved temp IDs