diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index b50dba29e3e..e2c7b9cd15f 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -31,19 +31,16 @@ const { parseDeduplicateByTitle, normalizeTitleForDedup, findDuplicateByTitle } * @returns {Object} An object containing all handler functions */ function createHandlers(server, appendSafeOutput, config = {}) { + const TOKEN_THRESHOLD = 16000; + /** - * Default handler for safe output tools - * Spec cross-reference: Safe Output Outcome Evaluation §2/§4/§5/§6/§7/§8/§9/§10/§11/§12/§13/§14/§15/§16/§18/§19/§20/§21/§22/§23/§24/§25/§26/§27/§28/§29. - * @param {string} type - The tool type - * @returns {Function} Handler function + * Detect and offload large string fields to files. + * @param {Record} entry + * @returns {Object | null} MCP response if large content was handled, else null */ - const defaultHandler = type => args => { - const entry = { ...(args || {}), type }; - - // Check if any field in the entry has content exceeding 16000 tokens + const maybeHandleLargeContent = entry => { let largeContent = null; let largeFieldName = null; - const TOKEN_THRESHOLD = 16000; for (const [key, value] of Object.entries(entry)) { if (typeof value === "string") { @@ -57,26 +54,34 @@ function createHandlers(server, appendSafeOutput, config = {}) { } } - if (largeContent && largeFieldName) { - // Write large content to file - const fileInfo = writeLargeContentToFile(largeContent); + if (!largeContent || !largeFieldName) { + return null; + } - // Replace large field with file reference - entry[largeFieldName] = `[Content too large, saved to file: ${fileInfo.filename}]`; + const fileInfo = writeLargeContentToFile(largeContent); + entry[largeFieldName] = `[Content too large, saved to file: ${fileInfo.filename}]`; + appendSafeOutput(entry); - // Append modified entry to safe outputs - appendSafeOutput(entry); + return { + content: [ + { + type: "text", + text: JSON.stringify(fileInfo), + }, + ], + }; + }; - // Return file info to the agent - return { - content: [ - { - type: "text", - text: JSON.stringify(fileInfo), - }, - ], - }; - } + /** + * Default handler for safe output tools + * Spec cross-reference: Safe Output Outcome Evaluation §2/§4/§5/§6/§7/§8/§9/§10/§11/§12/§13/§14/§15/§16/§18/§19/§20/§21/§22/§23/§24/§25/§26/§27/§28/§29. + * @param {string} type - The tool type + * @returns {Function} Handler function + */ + const defaultHandler = type => args => { + const entry = { ...(args || {}), type }; + const largeContentResponse = maybeHandleLargeContent(entry); + if (largeContentResponse) return largeContentResponse; // Normal case - no large content appendSafeOutput(entry); @@ -1008,7 +1013,10 @@ function createHandlers(server, appendSafeOutput, config = {}) { _duplicate_title: duplicate.title, _duplicate_distance: duplicate.distance, }; - appendSafeOutput(droppedEntry); + const largeContentResponse = maybeHandleLargeContent(droppedEntry); + if (!largeContentResponse) { + appendSafeOutput(droppedEntry); + } return { content: [ { @@ -1025,6 +1033,9 @@ function createHandlers(server, appendSafeOutput, config = {}) { seenIssueTitlesByRepo.set(resolvedRepo, seenTitles); } + const largeContentResponse = maybeHandleLargeContent(entry); + if (largeContentResponse) return largeContentResponse; + appendSafeOutput(entry); return { content: [ diff --git a/actions/setup/js/safe_outputs_handlers.test.cjs b/actions/setup/js/safe_outputs_handlers.test.cjs index c9fafb3a175..5d26205379d 100644 --- a/actions/setup/js/safe_outputs_handlers.test.cjs +++ b/actions/setup/js/safe_outputs_handlers.test.cjs @@ -4,6 +4,8 @@ import path from "path"; import { execSync } from "child_process"; import { createHandlers } from "./safe_outputs_handlers.cjs"; +const LARGE_CONTENT_BODY = "A".repeat(70000); + // Mock the global objects that GitHub Actions provides const mockCore = { debug: vi.fn(), @@ -1295,6 +1297,22 @@ describe("safe_outputs_handlers", () => { expect(secondResponse.result).toBe("duplicate_dropped"); }); + it("should offload large body when appending duplicate create_issue entry", () => { + const h = createHandlers(mockServer, mockAppendSafeOutput, { + create_issue: { + deduplicate_by_title: true, + }, + }); + + h.createIssueHandler({ title: "Duplicate Issue", body: "First body" }); + h.createIssueHandler({ title: "Duplicate Issue", body: LARGE_CONTENT_BODY }); + + expect(mockAppendSafeOutput.mock.calls).toHaveLength(2); + const droppedEntry = mockAppendSafeOutput.mock.calls[1][0]; + expect(droppedEntry._dropped_duplicate_by_title).toBe(true); + expect(droppedEntry.body).toContain("[Content too large, saved to file:"); + }); + it("should reject invalid deduplicate-by-title configuration", () => { expect(() => createHandlers(mockServer, mockAppendSafeOutput, {