Skip to content

Commit 269ae83

Browse files
Copilotpelikhan
andauthored
fix: silently skip add_comment when no triggering context on schedule runs (#24131)
* Initial plan * fix: silently skip add_comment when no triggering context (schedule runs) - add_comment.cjs: add `skipped: true` to max count reached return so exceeding the per-run comment limit is a graceful skip, not a failure - safe_output_handler_manager.cjs: log all skipped messages (not just code-push types) so add_comment skips are visible in CI logs; add a "Skipped (no context or limit reached)" summary line for transparency - add_comment.test.cjs: assert `skipped: true` on the max-count result The core fix (returning skipped:true when target is "triggering" but no issue/PR context exists, e.g. schedule runs) was already in place in add_comment.cjs + safe_output_helpers.cjs. These changes improve logging visibility and extend the skipped semantics to the max-count path. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/7679435a-c1b7-4a0c-b3d7-4204d76aafad Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> Co-authored-by: Peli de Halleux <pelikhan@users.noreply.github.com>
1 parent e92ff6b commit 269ae83

File tree

3 files changed

+9
-3
lines changed

3 files changed

+9
-3
lines changed

actions/setup/js/add_comment.cjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -330,6 +330,7 @@ async function main(config = {}) {
330330
core.warning(`Skipping add_comment: max count of ${maxCount} reached`);
331331
return {
332332
success: false,
333+
skipped: true,
333334
error: `Max count of ${maxCount} reached`,
334335
};
335336
}

actions/setup/js/add_comment.test.cjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2027,6 +2027,7 @@ describe("add_comment", () => {
20272027
expect(result1.success).toBe(true);
20282028
expect(result2.success).toBe(true);
20292029
expect(result3.success).toBe(false);
2030+
expect(result3.skipped).toBe(true);
20302031
expect(result3.error).toMatch(/max count/i);
20312032
});
20322033
});

actions/setup/js/safe_output_handler_manager.cjs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -481,9 +481,7 @@ async function processMessages(messageHandlers, messages, onItemCreated = null)
481481
// Skipped results should NOT trigger fail-fast cancellation of subsequent messages.
482482
if (result && result.success === false && result.skipped === true && !result.deferred) {
483483
const msg = result.error || "Handler returned success: false with skipped: true";
484-
if (CODE_PUSH_TYPES.has(messageType)) {
485-
core.info(`⏭ Message ${i + 1} (${messageType}) skipped — ${msg}`);
486-
}
484+
core.info(`⏭ Message ${i + 1} (${messageType}) skipped — ${msg}`);
487485
results.push({
488486
type: messageType,
489487
messageIndex: i,
@@ -1090,6 +1088,7 @@ async function main() {
10901088
const skippedStandaloneResults = processingResult.results.filter(r => r.skipped && r.reason === "Handled by standalone step");
10911089
const skippedCustomJobResults = processingResult.results.filter(r => r.skipped && r.reason === "Handled by custom safe output job");
10921090
const skippedNoHandlerResults = processingResult.results.filter(r => !r.success && !r.skipped && r.error?.includes("No handler loaded"));
1091+
const skippedHandlerResults = processingResult.results.filter(r => r.skipped && !r.reason && !r.deferred && !r.cancelled);
10931092

10941093
core.info(`\n=== Processing Summary ===`);
10951094
core.info(`Total messages: ${processingResult.results.length}`);
@@ -1101,6 +1100,11 @@ async function main() {
11011100
if (deferredCount > 0) {
11021101
core.info(`Deferred: ${deferredCount}`);
11031102
}
1103+
if (skippedHandlerResults.length > 0) {
1104+
core.info(`Skipped (no context or limit reached): ${skippedHandlerResults.length}`);
1105+
const skippedHandlerTypes = [...new Set(skippedHandlerResults.map(r => r.type))];
1106+
core.info(` Types: ${skippedHandlerTypes.join(", ")}`);
1107+
}
11041108
if (skippedStandaloneResults.length > 0) {
11051109
core.info(`Skipped (standalone step): ${skippedStandaloneResults.length}`);
11061110
const standaloneTypes = [...new Set(skippedStandaloneResults.map(r => r.type))];

0 commit comments

Comments
 (0)