diff --git a/.changeset/patch-enforce-add-comment-mcp.md b/.changeset/patch-enforce-add-comment-mcp.md new file mode 100644 index 0000000000..27d8acb1e6 --- /dev/null +++ b/.changeset/patch-enforce-add-comment-mcp.md @@ -0,0 +1,4 @@ +--- +"gh-aw": patch +--- +Enforce the add_comment constraints early by validating tool calls in the MCP server and reuse the shared helper for limit checks. diff --git a/actions/setup/js/add_comment.cjs b/actions/setup/js/add_comment.cjs index 8e5584270b..194648a234 100644 --- a/actions/setup/js/add_comment.cjs +++ b/actions/setup/js/add_comment.cjs @@ -15,49 +15,11 @@ const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_help const { getMissingInfoSections } = require("./missing_messages_helper.cjs"); const { getMessages } = require("./messages_core.cjs"); const { sanitizeContent } = require("./sanitize_content.cjs"); +const { MAX_COMMENT_LENGTH, MAX_MENTIONS, MAX_LINKS, enforceCommentLimits } = require("./comment_limit_helpers.cjs"); /** @type {string} Safe output type handled by this module */ const HANDLER_TYPE = "add_comment"; -/** - * Maximum limits for comment parameters to prevent resource exhaustion. - * These limits align with GitHub's API constraints and security best practices. - */ -/** @type {number} Maximum comment body length (GitHub's limit) */ -const MAX_COMMENT_LENGTH = 65536; - -/** @type {number} Maximum number of mentions allowed per comment */ -const MAX_MENTIONS = 10; - -/** @type {number} Maximum number of links allowed per comment */ -const MAX_LINKS = 50; - -/** - * Enforces maximum limits on comment parameters to prevent resource exhaustion attacks. - * Per Safe Outputs specification requirement MR3, limits must be enforced before API calls. - * - * @param {string} body - Comment body to validate - * @throws {Error} When any limit is exceeded, with error code and details - */ -function enforceCommentLimits(body) { - // Check body length - max limit exceeded check - if (body.length > MAX_COMMENT_LENGTH) { - throw new Error(`E006: Comment body exceeds maximum length of ${MAX_COMMENT_LENGTH} characters (got ${body.length})`); - } - - // Count mentions (@username pattern) - max limit exceeded check - const mentions = (body.match(/@\w+/g) || []).length; - if (mentions > MAX_MENTIONS) { - throw new Error(`E007: Comment contains ${mentions} mentions, maximum is ${MAX_MENTIONS}`); - } - - // Count links (http:// and https:// URLs) - max limit exceeded check - const links = (body.match(/https?:\/\/[^\s]+/g) || []).length; - if (links > MAX_LINKS) { - throw new Error(`E008: Comment contains ${links} links, maximum is ${MAX_LINKS}`); - } -} - // Copy helper functions from original file async function minimizeComment(github, nodeId, reason = "outdated") { const query = /* GraphQL */ ` diff --git a/actions/setup/js/comment_limit_helpers.cjs b/actions/setup/js/comment_limit_helpers.cjs new file mode 100644 index 0000000000..32eaa0f10b --- /dev/null +++ b/actions/setup/js/comment_limit_helpers.cjs @@ -0,0 +1,62 @@ +// @ts-check + +/** + * Comment Limit Helpers + * + * This module provides validation functions for enforcing comment constraints + * across both MCP server (Phase 4 - tool invocation) and safe output processor + * (Phase 6 - API execution) per Safe Outputs Specification MCE1 and MCE4. + * + * The limits defined here must match the constraints documented in: + * - pkg/workflow/js/safe_outputs_tools.json (add_comment tool description) + * - docs/src/content/docs/reference/safe-outputs-specification.md + * + * This ensures constraint consistency per Requirement MCE5. + */ + +/** + * Maximum limits for comment parameters to prevent resource exhaustion. + * These limits align with GitHub's API constraints and security best practices. + */ + +/** @type {number} Maximum comment body length (GitHub's limit) */ +const MAX_COMMENT_LENGTH = 65536; + +/** @type {number} Maximum number of mentions allowed per comment */ +const MAX_MENTIONS = 10; + +/** @type {number} Maximum number of links allowed per comment */ +const MAX_LINKS = 50; + +/** + * Enforces maximum limits on comment parameters to prevent resource exhaustion attacks. + * Per Safe Outputs specification requirement MR3, limits must be enforced before API calls. + * + * @param {string} body - Comment body to validate + * @throws {Error} When any limit is exceeded, with error code and details + */ +function enforceCommentLimits(body) { + // Check body length - max limit exceeded check + if (body.length > MAX_COMMENT_LENGTH) { + throw new Error(`E006: Comment body exceeds maximum length of ${MAX_COMMENT_LENGTH} characters (got ${body.length})`); + } + + // Count mentions (@username pattern) - max limit exceeded check + const mentions = (body.match(/@\w+/g) || []).length; + if (mentions > MAX_MENTIONS) { + throw new Error(`E007: Comment contains ${mentions} mentions, maximum is ${MAX_MENTIONS}`); + } + + // Count links (http:// and https:// URLs) - max limit exceeded check + const links = (body.match(/https?:\/\/[^\s]+/g) || []).length; + if (links > MAX_LINKS) { + throw new Error(`E008: Comment contains ${links} links, maximum is ${MAX_LINKS}`); + } +} + +module.exports = { + MAX_COMMENT_LENGTH, + MAX_MENTIONS, + MAX_LINKS, + enforceCommentLimits, +}; diff --git a/actions/setup/js/comment_limit_helpers.test.cjs b/actions/setup/js/comment_limit_helpers.test.cjs new file mode 100644 index 0000000000..e2d5c740f0 --- /dev/null +++ b/actions/setup/js/comment_limit_helpers.test.cjs @@ -0,0 +1,99 @@ +// @ts-check +import { describe, it, expect } from "vitest"; + +describe("comment_limit_helpers", () => { + let MAX_COMMENT_LENGTH, MAX_MENTIONS, MAX_LINKS, enforceCommentLimits; + + // Import the module + beforeEach(async () => { + const module = await import("./comment_limit_helpers.cjs"); + MAX_COMMENT_LENGTH = module.MAX_COMMENT_LENGTH; + MAX_MENTIONS = module.MAX_MENTIONS; + MAX_LINKS = module.MAX_LINKS; + enforceCommentLimits = module.enforceCommentLimits; + }); + + describe("constants", () => { + it("should export MAX_COMMENT_LENGTH constant", () => { + expect(MAX_COMMENT_LENGTH).toBe(65536); + }); + + it("should export MAX_MENTIONS constant", () => { + expect(MAX_MENTIONS).toBe(10); + }); + + it("should export MAX_LINKS constant", () => { + expect(MAX_LINKS).toBe(50); + }); + }); + + describe("enforceCommentLimits", () => { + it("should accept valid comment body", () => { + const validBody = "This is a valid comment with reasonable length"; + expect(() => enforceCommentLimits(validBody)).not.toThrow(); + }); + + it("should accept comment at exactly maximum length", () => { + const exactBody = "a".repeat(MAX_COMMENT_LENGTH); + expect(() => enforceCommentLimits(exactBody)).not.toThrow(); + }); + + it("should reject comment exceeding maximum length", () => { + const longBody = "a".repeat(MAX_COMMENT_LENGTH + 1); + expect(() => enforceCommentLimits(longBody)).toThrow(/E006.*maximum length/i); + }); + + it("should accept comment with exactly maximum mentions", () => { + const mentions = Array.from({ length: MAX_MENTIONS }, (_, i) => `@user${i}`).join(" "); + expect(() => enforceCommentLimits(mentions)).not.toThrow(); + }); + + it("should reject comment exceeding maximum mentions", () => { + const mentions = Array.from({ length: MAX_MENTIONS + 1 }, (_, i) => `@user${i}`).join(" "); + expect(() => enforceCommentLimits(mentions)).toThrow(/E007.*mentions/i); + }); + + it("should accept comment with exactly maximum links", () => { + const links = Array.from({ length: MAX_LINKS }, (_, i) => `https://example.com/${i}`).join(" "); + expect(() => enforceCommentLimits(links)).not.toThrow(); + }); + + it("should reject comment exceeding maximum links", () => { + const links = Array.from({ length: MAX_LINKS + 1 }, (_, i) => `https://example.com/${i}`).join(" "); + expect(() => enforceCommentLimits(links)).toThrow(/E008.*links/i); + }); + + it("should count both http and https links", () => { + const httpsLinks = Array.from({ length: 30 }, (_, i) => `https://example.com/${i}`).join(" "); + const httpLinks = Array.from({ length: 21 }, (_, i) => `http://example.org/${i}`).join(" "); + const mixed = `${httpsLinks} ${httpLinks}`; + expect(() => enforceCommentLimits(mixed)).toThrow(/E008.*51.*links/i); + }); + + it("should include actual values in error messages", () => { + const longBody = "a".repeat(70000); + try { + enforceCommentLimits(longBody); + expect.fail("Should have thrown error"); + } catch (error) { + expect(error.message).toContain("E006"); + expect(error.message).toContain("65536"); // Max length + expect(error.message).toContain("70000"); // Actual length + } + }); + + it("should accept empty string", () => { + expect(() => enforceCommentLimits("")).not.toThrow(); + }); + + it("should handle comment with no mentions", () => { + const body = "Comment without any user mentions"; + expect(() => enforceCommentLimits(body)).not.toThrow(); + }); + + it("should handle comment with no links", () => { + const body = "Comment without any links or URLs"; + expect(() => enforceCommentLimits(body)).not.toThrow(); + }); + }); +}); diff --git a/actions/setup/js/safe_outputs_handlers.cjs b/actions/setup/js/safe_outputs_handlers.cjs index 5e3425b9f4..6c045433a3 100644 --- a/actions/setup/js/safe_outputs_handlers.cjs +++ b/actions/setup/js/safe_outputs_handlers.cjs @@ -10,6 +10,8 @@ const { writeLargeContentToFile } = require("./write_large_content_to_file.cjs") const { getCurrentBranch } = require("./get_current_branch.cjs"); const { getBaseBranch } = require("./get_base_branch.cjs"); const { generateGitPatch } = require("./generate_git_patch.cjs"); +const { enforceCommentLimits } = require("./comment_limit_helpers.cjs"); +const { getErrorMessage } = require("./error_helpers.cjs"); /** * Create handlers for safe output tools @@ -372,12 +374,38 @@ function createHandlers(server, appendSafeOutput, config = {}) { }; }; + /** + * Handler for add_comment tool + * Per Safe Outputs Specification MCE1: Enforces constraints during tool invocation + * to provide immediate feedback to the LLM before recording to NDJSON + */ + const addCommentHandler = args => { + // Validate comment constraints before appending to safe outputs + // This provides early feedback per Requirement MCE1 (Early Validation) + try { + const body = args.body || ""; + enforceCommentLimits(body); + } catch (error) { + // Return validation error with specific constraint violation details + // Per Requirement MCE3 (Actionable Error Responses) + // Use JSON-RPC error code -32602 (Invalid params) per MCP specification + throw { + code: -32602, + message: getErrorMessage(error), + }; + } + + // If validation passes, record the operation using default handler + return defaultHandler("add_comment")(args); + }; + return { defaultHandler, uploadAssetHandler, createPullRequestHandler, pushToPullRequestBranchHandler, createProjectHandler, + addCommentHandler, }; } diff --git a/actions/setup/js/safe_outputs_mcp_add_comment_constraints.test.cjs b/actions/setup/js/safe_outputs_mcp_add_comment_constraints.test.cjs new file mode 100644 index 0000000000..41b83ec155 --- /dev/null +++ b/actions/setup/js/safe_outputs_mcp_add_comment_constraints.test.cjs @@ -0,0 +1,482 @@ +// @ts-check +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import fs from "fs"; +import path from "path"; +import os from "os"; + +/** + * Tests for MCP Server Constraint Enforcement - add_comment tool + * + * This test suite validates Requirement MCE1 (Early Validation) from the Safe Outputs + * Specification Section 8.3: MCP servers must enforce operational constraints during + * tool invocation (Phase 4) to provide immediate feedback to the LLM. + * + * Tests verify that: + * 1. Valid comments pass through successfully + * 2. E006 error is returned for length violations (>65536 chars) + * 3. E007 error is returned for mention violations (>10 mentions) + * 4. E008 error is returned for link violations (>50 links) + * 5. Errors use MCP JSON-RPC error format with code -32602 + * 6. Error messages are actionable per Requirement MCE3 + * + * Related issues: github/gh-aw# + */ + +describe("Safe Outputs MCP Server - add_comment Constraint Enforcement", () => { + let createServer, registerTool, handleRequest; + let server; + let tempDir; + let configFile; + let outputFile; + let toolsFile; + + beforeEach(async () => { + vi.resetModules(); + + // Suppress stderr output during tests + vi.spyOn(process.stderr, "write").mockImplementation(() => true); + + // Create temporary directory for test files + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "mcp-add-comment-test-")); + configFile = path.join(tempDir, "config.json"); + outputFile = path.join(tempDir, "output.jsonl"); + toolsFile = path.join(tempDir, "tools.json"); + + // Create config file enabling add_comment + const config = { + "add-comment": { enabled: true }, + }; + fs.writeFileSync(configFile, JSON.stringify(config)); + + // Load tools schema with add_comment + const toolsPath = path.join(process.cwd(), "safe_outputs_tools.json"); + const toolsContent = fs.readFileSync(toolsPath, "utf8"); + const allTools = JSON.parse(toolsContent); + const addCommentTool = allTools.find(t => t.name === "add_comment"); + if (!addCommentTool) { + throw new Error("add_comment tool not found in safe_outputs_tools.json"); + } + fs.writeFileSync(toolsFile, JSON.stringify([addCommentTool])); + + // Set environment variables + process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH = configFile; + process.env.GH_AW_SAFE_OUTPUTS_OUTPUT_PATH = outputFile; + process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH = toolsFile; + + // Import MCP server modules + const mcpCore = await import("./mcp_server_core.cjs"); + createServer = mcpCore.createServer; + registerTool = mcpCore.registerTool; + handleRequest = mcpCore.handleRequest; + + // Import and set up handlers + const handlersModule = await import("./safe_outputs_handlers.cjs"); + const appendModule = await import("./safe_outputs_append.cjs"); + const toolsLoaderModule = await import("./safe_outputs_tools_loader.cjs"); + + // Create server + server = createServer({ name: "test-safeoutputs", version: "1.0.0" }); + + // Create append function + const appendSafeOutput = appendModule.createAppendFunction(outputFile); + + // Create handlers + const handlers = handlersModule.createHandlers(server, appendSafeOutput, config); + + // Load and attach handlers to tools + const tools = toolsLoaderModule.loadTools(server); + const toolsWithHandlers = toolsLoaderModule.attachHandlers(tools, handlers); + + // Register tools + toolsWithHandlers.forEach(tool => { + registerTool(server, tool); + }); + }); + + afterEach(() => { + // Clean up temp directory + if (tempDir && fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + + // Clean up environment variables + delete process.env.GH_AW_SAFE_OUTPUTS_CONFIG_PATH; + delete process.env.GH_AW_SAFE_OUTPUTS_OUTPUT_PATH; + delete process.env.GH_AW_SAFE_OUTPUTS_TOOLS_PATH; + }); + + describe("Valid Comments", () => { + it("should accept comment with valid body", async () => { + const request = { + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: "This is a valid comment with reasonable length", + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).toHaveProperty("jsonrpc", "2.0"); + expect(response).toHaveProperty("id", 1); + expect(response).toHaveProperty("result"); + expect(response).not.toHaveProperty("error"); + expect(response.result.content).toBeDefined(); + expect(response.result.content[0].text).toContain("success"); + }); + + it("should accept comment at exactly maximum length (65536 chars)", async () => { + const maxBody = "a".repeat(65536); + const request = { + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: maxBody, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).not.toHaveProperty("error"); + // Large content (>16000 tokens) may return a file reference instead of "success" + expect(response.result.content[0].text).toBeTruthy(); + }); + + it("should accept comment with exactly 10 mentions", async () => { + const mentions = Array.from({ length: 10 }, (_, i) => `@user${i}`).join(" "); + const request = { + jsonrpc: "2.0", + id: 3, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Valid comment with mentions: ${mentions}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).not.toHaveProperty("error"); + expect(response.result.content[0].text).toContain("success"); + }); + + it("should accept comment with exactly 50 links", async () => { + const links = Array.from({ length: 50 }, (_, i) => `https://example.com/${i}`).join(" "); + const request = { + jsonrpc: "2.0", + id: 4, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Valid comment with links: ${links}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).not.toHaveProperty("error"); + expect(response.result.content[0].text).toContain("success"); + }); + }); + + describe("E006: Comment Length Violations", () => { + it("should reject comment exceeding maximum length", async () => { + const longBody = "a".repeat(65537); // One character over limit + const request = { + jsonrpc: "2.0", + id: 5, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: longBody, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).toHaveProperty("jsonrpc", "2.0"); + expect(response).toHaveProperty("id", 5); + expect(response).toHaveProperty("error"); + expect(response.error.code).toBe(-32602); // Invalid params + expect(response.error.message).toMatch(/E006/); + expect(response.error.message).toMatch(/65536/); // Max length + expect(response.error.message).toMatch(/65537/); // Actual length + }); + + it("should provide actionable error message for length violation", async () => { + const longBody = "a".repeat(70000); + const request = { + jsonrpc: "2.0", + id: 6, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: longBody, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response.error.message).toContain("maximum length"); + expect(response.error.message).toMatch(/\d+/); // Contains actual count + }); + }); + + describe("E007: Mention Count Violations", () => { + it("should reject comment with too many mentions", async () => { + const mentions = Array.from({ length: 11 }, (_, i) => `@user${i}`).join(" "); + const request = { + jsonrpc: "2.0", + id: 7, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Comment with too many mentions: ${mentions}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).toHaveProperty("error"); + expect(response.error.code).toBe(-32602); + expect(response.error.message).toMatch(/E007/); + expect(response.error.message).toMatch(/11/); // Actual count + expect(response.error.message).toMatch(/10/); // Maximum + }); + + it("should count mentions correctly in markdown", async () => { + const mentions = "@alice @bob @charlie @david @eve @frank @grace @henry @iris @jack @kate"; + const request = { + jsonrpc: "2.0", + id: 8, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Team review: ${mentions}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).toHaveProperty("error"); + expect(response.error.message).toMatch(/E007/); + expect(response.error.message).toContain("mentions"); + }); + }); + + describe("E008: Link Count Violations", () => { + it("should reject comment with too many links", async () => { + const links = Array.from({ length: 51 }, (_, i) => `https://example.com/${i}`).join(" "); + const request = { + jsonrpc: "2.0", + id: 9, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Comment with too many links: ${links}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).toHaveProperty("error"); + expect(response.error.code).toBe(-32602); + expect(response.error.message).toMatch(/E008/); + expect(response.error.message).toMatch(/51/); // Actual count + expect(response.error.message).toMatch(/50/); // Maximum + }); + + it("should count both http and https links", async () => { + const httpsLinks = Array.from({ length: 30 }, (_, i) => `https://example.com/${i}`).join(" "); + const httpLinks = Array.from({ length: 21 }, (_, i) => `http://example.org/${i}`).join(" "); + const request = { + jsonrpc: "2.0", + id: 10, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Mixed links: ${httpsLinks} ${httpLinks}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + expect(response).toHaveProperty("error"); + expect(response.error.message).toMatch(/E008/); + expect(response.error.message).toMatch(/51/); // 30 + 21 = 51 + }); + }); + + describe("MCP Error Format Compliance", () => { + it("should use JSON-RPC error format with code -32602", async () => { + const longBody = "a".repeat(70000); + const request = { + jsonrpc: "2.0", + id: 11, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: longBody, + }, + }, + }; + + const response = await handleRequest(server, request); + + // Validate JSON-RPC 2.0 error response structure + expect(response).toHaveProperty("jsonrpc", "2.0"); + expect(response).toHaveProperty("id", 11); + expect(response).toHaveProperty("error"); + expect(response).not.toHaveProperty("result"); + expect(response.error).toHaveProperty("code"); + expect(response.error).toHaveProperty("message"); + expect(response.error.code).toBe(-32602); + }); + + it("should provide specific constraint violation details in message", async () => { + const mentions = Array.from({ length: 15 }, (_, i) => `@user${i}`).join(" "); + const request = { + jsonrpc: "2.0", + id: 12, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: `Too many: ${mentions}`, + }, + }, + }; + + const response = await handleRequest(server, request); + + // Error message should be actionable per MCE3 + expect(response.error.message).toMatch(/mentions/i); + expect(response.error.message).toMatch(/15/); // Actual value + expect(response.error.message).toMatch(/10/); // Limit + }); + }); + + describe("Empty and Edge Cases", () => { + it("should reject empty body (body is required)", async () => { + const request = { + jsonrpc: "2.0", + id: 13, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: "", + }, + }, + }; + + const response = await handleRequest(server, request); + + // Empty body should be rejected by schema validation + expect(response).toHaveProperty("error"); + expect(response.error.code).toBe(-32602); + }); + + it("should handle missing body gracefully", async () => { + const request = { + jsonrpc: "2.0", + id: 14, + method: "tools/call", + params: { + name: "add_comment", + arguments: {}, + }, + }; + + const response = await handleRequest(server, request); + + // Should either succeed with empty body or require body field + // depending on schema requirements + expect(response).toHaveProperty("jsonrpc", "2.0"); + }); + }); + + describe("Dual Enforcement (MCE4)", () => { + it("should record operation to NDJSON only after validation passes", async () => { + const validRequest = { + jsonrpc: "2.0", + id: 15, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: "Valid comment", + }, + }, + }; + + await handleRequest(server, validRequest); + + // Check NDJSON file was created and contains the operation + expect(fs.existsSync(outputFile)).toBe(true); + const content = fs.readFileSync(outputFile, "utf8"); + const lines = content.trim().split("\n"); + expect(lines.length).toBeGreaterThan(0); + + const lastLine = JSON.parse(lines[lines.length - 1]); + expect(lastLine.type).toBe("add_comment"); + expect(lastLine.body).toBe("Valid comment"); + }); + + it("should NOT record operation to NDJSON when validation fails", async () => { + const invalidRequest = { + jsonrpc: "2.0", + id: 16, + method: "tools/call", + params: { + name: "add_comment", + arguments: { + body: "a".repeat(70000), + }, + }, + }; + + await handleRequest(server, invalidRequest); + + // NDJSON file should either not exist or not contain the invalid operation + if (fs.existsSync(outputFile)) { + const content = fs.readFileSync(outputFile, "utf8"); + if (content.trim()) { + const lines = content.trim().split("\n"); + lines.forEach(line => { + const entry = JSON.parse(line); + // Should not have recorded the too-long body + if (entry.type === "add_comment") { + expect(entry.body.length).toBeLessThanOrEqual(65536); + } + }); + } + } + }); + }); +}); diff --git a/actions/setup/js/safe_outputs_tools_loader.cjs b/actions/setup/js/safe_outputs_tools_loader.cjs index 56df445b7d..47201c65e9 100644 --- a/actions/setup/js/safe_outputs_tools_loader.cjs +++ b/actions/setup/js/safe_outputs_tools_loader.cjs @@ -57,6 +57,7 @@ function attachHandlers(tools, handlers) { push_to_pull_request_branch: handlers.pushToPullRequestBranchHandler, upload_asset: handlers.uploadAssetHandler, create_project: handlers.createProjectHandler, + add_comment: handlers.addCommentHandler, }; tools.forEach(tool => { diff --git a/actions/setup/setup.sh b/actions/setup/setup.sh index 9ecf24beb3..162ff11cc2 100755 --- a/actions/setup/setup.sh +++ b/actions/setup/setup.sh @@ -214,6 +214,7 @@ SAFE_OUTPUTS_FILES=( "error_helpers.cjs" "git_helpers.cjs" "mcp_enhanced_errors.cjs" + "comment_limit_helpers.cjs" ) SAFE_OUTPUTS_COUNT=0 diff --git a/docs/src/content/docs/reference/safe-outputs-specification.md b/docs/src/content/docs/reference/safe-outputs-specification.md index 45d9c61883..d6f23c58a0 100644 --- a/docs/src/content/docs/reference/safe-outputs-specification.md +++ b/docs/src/content/docs/reference/safe-outputs-specification.md @@ -7,9 +7,9 @@ sidebar: # Safe Outputs MCP Gateway Specification -**Version**: 1.11.0 +**Version**: 1.12.0 **Status**: Working Draft -**Publication Date**: 2026-02-15 +**Publication Date**: 2026-02-16 **Editor**: GitHub Agentic Workflows Team **This Version**: [safe-outputs-specification](/gh-aw/reference/safe-outputs-specification/) **Latest Published Version**: This document @@ -3635,6 +3635,13 @@ safe-outputs: ## Appendix F: Document History +**Version 1.12.0** (2026-02-16): +- **Implemented**: MCE1 (Early Validation) for add_comment tool with MCP server constraint enforcement +- **Added**: Runtime validation in safe_outputs_handlers.cjs that enforces comment limits during tool invocation +- **Verified**: Dual enforcement pattern now operational - MCP server validates during Phase 4, safe output processor validates during Phase 6 +- **Enhanced**: Error responses now use JSON-RPC error code -32602 with actionable messages containing specific constraint details +- **Tested**: Comprehensive test suite (16 test cases) validates E006/E007/E008 error handling and MCP error format compliance + **Version 1.11.0** (2026-02-15): - **Added**: Section 8.3 "MCP Server Constraint Enforcement" specifying requirements for early validation during tool invocation (MCE1-MCE5) - **Enhanced**: Tool descriptions to surface operational constraints to the LLM (e.g., add_comment mention/link/length limits)