diff --git a/.github/workflows/smoke-copilot.lock.yml b/.github/workflows/smoke-copilot.lock.yml index 26db091cc4..bb8eb9abb8 100644 --- a/.github/workflows/smoke-copilot.lock.yml +++ b/.github/workflows/smoke-copilot.lock.yml @@ -27,7 +27,7 @@ # - shared/github-queries-safe-input.md # - shared/reporting.md # -# frontmatter-hash: 23fb974caf81e30df69d8ee71fb9c8a011690ef75670f3f539f699673640002e +# frontmatter-hash: a289c1143b17a3921c5895f2f2c184d1bf701c6673bd8f849f42535cacc9fb0c name: "Smoke Copilot" "on": @@ -37,7 +37,7 @@ name: "Smoke Copilot" types: - labeled schedule: - - cron: "46 */12 * * *" + - cron: "37 */12 * * *" workflow_dispatch: null permissions: {} @@ -304,7 +304,7 @@ jobs: mkdir -p /tmp/gh-aw/safeoutputs mkdir -p /tmp/gh-aw/mcp-logs/safeoutputs cat > /opt/gh-aw/safeoutputs/config.json << 'GH_AW_SAFE_OUTPUTS_CONFIG_EOF' - {"add_comment":{"allowed_repos":["github/gh-aw"],"max":2},"add_labels":{"allowed":["smoke-copilot"],"allowed_repos":["github/gh-aw"],"max":3},"create_issue":{"expires":2,"group":true,"max":1},"dispatch_workflow":{"max":1,"workflow_files":{"haiku-printer":".yml"},"workflows":["haiku-printer"]},"missing_data":{},"missing_tool":{},"noop":{"max":1},"remove_labels":{"allowed":["smoke"],"max":3},"send-slack-message":{"description":"Send a message to Slack (stub for testing)","inputs":{"message":{"default":null,"description":"The message to send","required":true,"type":"string"}},"output":"Slack message stub executed!"}} + {"add_comment":{"allowed_repos":["github/gh-aw"],"max":2},"add_labels":{"allowed":["smoke-copilot"],"allowed_repos":["github/gh-aw"],"max":3},"create_issue":{"expires":2,"group":true,"max":1},"create_pull_request_review_comment":{"max":5},"dispatch_workflow":{"max":1,"workflow_files":{"haiku-printer":".yml"},"workflows":["haiku-printer"]},"missing_data":{},"missing_tool":{},"noop":{"max":1},"remove_labels":{"allowed":["smoke"],"max":3},"send-slack-message":{"description":"Send a message to Slack (stub for testing)","inputs":{"message":{"default":null,"description":"The message to send","required":true,"type":"string"}},"output":"Slack message stub executed!"},"submit_pull_request_review":{"max":1}} GH_AW_SAFE_OUTPUTS_CONFIG_EOF cat > /opt/gh-aw/safeoutputs/tools.json << 'GH_AW_SAFE_OUTPUTS_TOOLS_EOF' [ @@ -373,6 +373,74 @@ jobs: }, "name": "add_comment" }, + { + "description": "Create a review comment on a specific line of code in a pull request. Use this for inline code review feedback, suggestions, or questions about specific code changes. For general PR comments not tied to specific lines, use add_comment instead. CONSTRAINTS: Maximum 5 review comment(s) can be created. Comments will be on the RIGHT side of the diff.", + "inputSchema": { + "additionalProperties": false, + "properties": { + "body": { + "description": "Review comment content in Markdown. Provide specific, actionable feedback about the code at this location.", + "type": "string" + }, + "line": { + "description": "Line number for the comment. For single-line comments, this is the target line. For multi-line comments, this is the ending line.", + "type": [ + "number", + "string" + ] + }, + "path": { + "description": "File path relative to the repository root (e.g., 'src/auth/login.js'). Must be a file that was changed in the PR.", + "type": "string" + }, + "side": { + "description": "Side of the diff to comment on: RIGHT for the new version (additions), LEFT for the old version (deletions). Defaults to RIGHT.", + "enum": [ + "LEFT", + "RIGHT" + ], + "type": "string" + }, + "start_line": { + "description": "Starting line number for multi-line comments. When set, the comment spans from start_line to line. Omit for single-line comments.", + "type": [ + "number", + "string" + ] + } + }, + "required": [ + "path", + "line", + "body" + ], + "type": "object" + }, + "name": "create_pull_request_review_comment" + }, + { + "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Use APPROVE to approve the PR, REQUEST_CHANGES to request changes, or COMMENT for general feedback without a decision. If you don't call this tool, review comments are still submitted as a COMMENT review. CONSTRAINTS: Maximum 1 review(s) can be submitted.", + "inputSchema": { + "additionalProperties": false, + "properties": { + "body": { + "description": "Overall review summary in Markdown. Provide a high-level assessment of the changes. Required for APPROVE and REQUEST_CHANGES events; optional for COMMENT.", + "type": "string" + }, + "event": { + "description": "Review decision: APPROVE to approve the pull request, REQUEST_CHANGES to formally request changes before merging, or COMMENT for general feedback without a formal decision. Defaults to COMMENT when omitted.", + "enum": [ + "APPROVE", + "REQUEST_CHANGES", + "COMMENT" + ], + "type": "string" + } + }, + "type": "object" + }, + "name": "submit_pull_request_review" + }, { "description": "Add labels to an existing GitHub issue or pull request for categorization and filtering. Labels must already exist in the repository. For creating new issues with labels, use create_issue with the labels property instead. CONSTRAINTS: Only these labels are allowed: [smoke-copilot].", "inputSchema": { @@ -592,6 +660,36 @@ jobs: } } }, + "create_pull_request_review_comment": { + "defaultMax": 1, + "fields": { + "body": { + "required": true, + "type": "string", + "sanitize": true, + "maxLength": 65000 + }, + "line": { + "required": true, + "positiveInteger": true + }, + "path": { + "required": true, + "type": "string" + }, + "side": { + "type": "string", + "enum": [ + "LEFT", + "RIGHT" + ] + }, + "start_line": { + "optionalPositiveInteger": true + } + }, + "customValidation": "startLineLessOrEqualLine" + }, "missing_tool": { "defaultMax": 20, "fields": { @@ -623,6 +721,24 @@ jobs: "maxLength": 65000 } } + }, + "submit_pull_request_review": { + "defaultMax": 1, + "fields": { + "body": { + "type": "string", + "sanitize": true, + "maxLength": 65000 + }, + "event": { + "type": "string", + "enum": [ + "APPROVE", + "REQUEST_CHANGES", + "COMMENT" + ] + } + } } } GH_AW_SAFE_OUTPUTS_VALIDATION_EOF @@ -1902,7 +2018,7 @@ jobs: uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8 env: GH_AW_AGENT_OUTPUT: ${{ env.GH_AW_AGENT_OUTPUT }} - GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"allowed_repos\":[\"github/gh-aw\"],\"hide_older_comments\":true,\"max\":2},\"add_labels\":{\"allowed\":[\"smoke-copilot\"],\"allowed_repos\":[\"github/gh-aw\"]},\"create_issue\":{\"close_older_issues\":true,\"expires\":2,\"group\":true,\"max\":1},\"dispatch_workflow\":{\"max\":1,\"workflow_files\":{\"haiku-printer\":\".yml\"},\"workflows\":[\"haiku-printer\"]},\"missing_data\":{},\"missing_tool\":{},\"remove_labels\":{\"allowed\":[\"smoke\"]}}" + GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: "{\"add_comment\":{\"allowed_repos\":[\"github/gh-aw\"],\"hide_older_comments\":true,\"max\":2},\"add_labels\":{\"allowed\":[\"smoke-copilot\"],\"allowed_repos\":[\"github/gh-aw\"]},\"create_issue\":{\"close_older_issues\":true,\"expires\":2,\"group\":true,\"max\":1},\"create_pull_request_review_comment\":{\"max\":5,\"side\":\"RIGHT\"},\"dispatch_workflow\":{\"max\":1,\"workflow_files\":{\"haiku-printer\":\".yml\"},\"workflows\":[\"haiku-printer\"]},\"missing_data\":{},\"missing_tool\":{},\"remove_labels\":{\"allowed\":[\"smoke\"]},\"submit_pull_request_review\":{\"max\":1}}" with: github-token: ${{ secrets.GH_AW_GITHUB_TOKEN || secrets.GITHUB_TOKEN }} script: | diff --git a/.github/workflows/smoke-copilot.md b/.github/workflows/smoke-copilot.md index 33e1dca482..b58432f79c 100644 --- a/.github/workflows/smoke-copilot.md +++ b/.github/workflows/smoke-copilot.md @@ -54,6 +54,9 @@ safe-outputs: expires: 2h group: true close-older-issues: true + create-pull-request-review-comment: + max: 5 + submit-pull-request-review: add-labels: allowed: [smoke-copilot] allowed-repos: ["github/gh-aw"] @@ -121,6 +124,7 @@ strict: true - Use the `add_comment` tool with `discussion_number: ` to add a fun, playful comment stating that the smoke test agent was here 8. **Build gh-aw**: Run `GOCACHE=/tmp/go-cache GOMODCACHE=/tmp/go-mod make build` to verify the agent can successfully build the gh-aw project (both caches must be set to /tmp because the default cache locations are not writable). If the command fails, mark this test as ❌ and report the failure. 9. **Workflow Dispatch Testing**: Use the `dispatch_workflow` safe output tool to trigger the `haiku-printer` workflow with a haiku as the message input. Create an original, creative haiku about software testing or automation. +10. **PR Review Testing**: Review the diff of the current pull request. Leave 1-2 inline `create_pull_request_review_comment` comments on specific lines, then call `submit_pull_request_review` with a brief body summarizing your review and event `COMMENT`. ## Output diff --git a/actions/setup/js/create_pr_review_comment.cjs b/actions/setup/js/create_pr_review_comment.cjs index 06e9cb3522..38348b6a2e 100644 --- a/actions/setup/js/create_pr_review_comment.cjs +++ b/actions/setup/js/create_pr_review_comment.cjs @@ -5,8 +5,6 @@ * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction */ -const { generateFooter } = require("./generate_footer.cjs"); -const { getRepositoryUrl } = require("./get_repository_url.cjs"); const { getErrorMessage } = require("./error_helpers.cjs"); const { resolveTargetRepoConfig, resolveAndValidateRepo } = require("./repo_helpers.cjs"); @@ -15,7 +13,10 @@ const HANDLER_TYPE = "create_pull_request_review_comment"; /** * Main handler factory for create_pull_request_review_comment - * Returns a message handler function that processes individual review comment messages + * Returns a message handler function that validates and buffers individual review comments. + * Comments are buffered in the PR review buffer (passed via config._prReviewBuffer) and + * submitted as a single PR review after all messages have been processed. + * * @type {HandlerFactoryFunction} */ async function main(config = {}) { @@ -23,8 +24,16 @@ async function main(config = {}) { const defaultSide = config.side || "RIGHT"; const commentTarget = config.target || "triggering"; const maxCount = config.max || 10; + const buffer = config._prReviewBuffer; const { defaultTargetRepo, allowedRepos } = resolveTargetRepoConfig(config); + if (!buffer) { + core.warning("create_pull_request_review_comment: No PR review buffer provided in config"); + return async function handleCreatePRReviewComment() { + return { success: false, error: "No PR review buffer available" }; + }; + } + core.info(`PR review comment target configuration: ${commentTarget}`); core.info(`Default comment side configuration: ${defaultSide}`); core.info(`Max count: ${maxCount}`); @@ -36,16 +45,31 @@ async function main(config = {}) { // Track how many items we've processed for max limit let processedCount = 0; - // Track created comments for outputs - const createdComments = []; - // Extract triggering context for footer generation const triggeringIssueNumber = context.payload?.issue?.number && !context.payload?.issue?.pull_request ? context.payload.issue.number : undefined; const triggeringPRNumber = context.payload?.pull_request?.number || (context.payload?.issue?.pull_request ? context.payload.issue.number : undefined); const triggeringDiscussionNumber = context.payload?.discussion?.number; + // Set footer context once for the review buffer + const workflowName = process.env.GH_AW_WORKFLOW_NAME || "Workflow"; + const workflowSource = process.env.GH_AW_WORKFLOW_SOURCE || ""; + const workflowSourceURL = process.env.GH_AW_WORKFLOW_SOURCE_URL || ""; + const runId = context.runId; + const githubServer = process.env.GITHUB_SERVER_URL || "https://github.com"; + const runUrl = context.payload.repository ? `${context.payload.repository.html_url}/actions/runs/${runId}` : `${githubServer}/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId}`; + + buffer.setFooterContext({ + workflowName, + runUrl, + workflowSource, + workflowSourceURL, + triggeringIssueNumber, + triggeringPRNumber, + triggeringDiscussionNumber, + }); + /** - * Message handler function that processes a single create_pull_request_review_comment message + * Message handler function that validates and buffers a single create_pull_request_review_comment message * @param {Object} message - The create_pull_request_review_comment message to process * @param {Object} resolvedTemporaryIds - Map of temporary IDs to {repo, number} * @returns {Promise} Result with success/error status and comment details @@ -203,8 +227,6 @@ async function main(config = {}) { }; } - core.info(`Creating review comment on PR #${pullRequestNumber}`); - // Parse line numbers const line = parseInt(commentItem.line, 10); if (isNaN(line) || line <= 0) { @@ -237,61 +259,47 @@ async function main(config = {}) { }; } - // Extract body from the JSON item - let body = commentItem.body.trim(); - - // Add AI disclaimer with workflow name and run url - const workflowName = process.env.GH_AW_WORKFLOW_NAME || "Workflow"; - const workflowSource = process.env.GH_AW_WORKFLOW_SOURCE || ""; - const workflowSourceURL = process.env.GH_AW_WORKFLOW_SOURCE_URL || ""; - const runId = context.runId; - const githubServer = process.env.GITHUB_SERVER_URL || "https://github.com"; - const runUrl = context.payload.repository ? `${context.payload.repository.html_url}/actions/runs/${runId}` : `${githubServer}/${context.repo.owner}/${context.repo.repo}/actions/runs/${runId}`; - body += generateFooter(workflowName, runUrl, workflowSource, workflowSourceURL, triggeringIssueNumber, triggeringPRNumber, triggeringDiscussionNumber); - - core.info(`Creating review comment on PR #${pullRequestNumber} in ${itemRepo} at ${commentItem.path}:${line}${startLine ? ` (lines ${startLine}-${line})` : ""} [${side}]`); - core.info(`Comment content length: ${body.length}`); - - try { - // Prepare the request parameters - /** @type {any} */ - const requestParams = { - owner: repoParts.owner, - repo: repoParts.repo, - pull_number: pullRequestNumber, - body: body, - path: commentItem.path, - commit_id: pullRequest && pullRequest.head ? pullRequest.head.sha : "", // Required for creating review comments - line: line, - side: side, + // Set the review context (first comment sets it) + // Reject comments targeting a different repo/PR than the first comment + const existingCtx = buffer.getReviewContext(); + if (existingCtx && (existingCtx.repo !== itemRepo || existingCtx.pullRequestNumber !== pullRequestNumber)) { + core.warning(`Skipping review comment: targets ${itemRepo}#${pullRequestNumber} but buffer is bound to ${existingCtx.repo}#${existingCtx.pullRequestNumber}. ` + "All review comments in a single review must target the same PR."); + return { + success: false, + error: `Review comments must target the same PR (buffer is bound to ${existingCtx.repo}#${existingCtx.pullRequestNumber})`, }; + } - // Add start_line for multi-line comments - if (startLine !== undefined) { - requestParams.start_line = startLine; - requestParams.start_side = side; // start_side should match side for consistency - } + buffer.setReviewContext({ + repo: itemRepo, + repoParts: repoParts, + pullRequestNumber: pullRequestNumber, + pullRequest: pullRequest, + }); + + // Buffer the comment instead of posting it individually + /** @type {import('./pr_review_buffer.cjs').BufferedComment} */ + const bufferedComment = { + path: commentItem.path, + line: line, + body: commentItem.body.trim(), + side: side, + }; + + if (startLine !== undefined) { + bufferedComment.start_line = startLine; + } - // Create the review comment using GitHub API - const { data: comment } = await github.rest.pulls.createReviewComment(requestParams); + buffer.addComment(bufferedComment); - core.info("Created review comment #" + comment.id + ": " + comment.html_url); - createdComments.push(comment); + core.info(`Buffered review comment on PR #${pullRequestNumber} in ${itemRepo} at ${commentItem.path}:${line}${startLine ? ` (lines ${startLine}-${line})` : ""} [${side}]`); - return { - success: true, - comment_id: comment.id, - comment_url: comment.html_url, - pull_request_number: pullRequestNumber, - repo: itemRepo, - }; - } catch (error) { - core.error(`✗ Failed to create review comment: ${getErrorMessage(error)}`); - return { - success: false, - error: getErrorMessage(error), - }; - } + return { + success: true, + buffered: true, + pull_request_number: pullRequestNumber, + repo: itemRepo, + }; }; } diff --git a/actions/setup/js/create_pr_review_comment.test.cjs b/actions/setup/js/create_pr_review_comment.test.cjs index bb26740db3..0e35baffe9 100644 --- a/actions/setup/js/create_pr_review_comment.test.cjs +++ b/actions/setup/js/create_pr_review_comment.test.cjs @@ -1,286 +1,475 @@ -import { describe, it, expect, beforeEach, vi } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import fs from "fs"; import path from "path"; + const mockCore = { - debug: vi.fn(), - info: vi.fn(), - notice: vi.fn(), - warning: vi.fn(), - error: vi.fn(), - setFailed: vi.fn(), - setOutput: vi.fn(), - exportVariable: vi.fn(), - setSecret: vi.fn(), - getInput: vi.fn(), - getBooleanInput: vi.fn(), - getMultilineInput: vi.fn(), - getState: vi.fn(), - saveState: vi.fn(), - startGroup: vi.fn(), - endGroup: vi.fn(), - group: vi.fn(), - addPath: vi.fn(), - setCommandEcho: vi.fn(), - isDebug: vi.fn().mockReturnValue(!1), - getIDToken: vi.fn(), - toPlatformPath: vi.fn(), - toPosixPath: vi.fn(), - toWin32Path: vi.fn(), - summary: { addRaw: vi.fn().mockReturnThis(), write: vi.fn().mockResolvedValue() }, + debug: vi.fn(), + info: vi.fn(), + notice: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + exportVariable: vi.fn(), + setSecret: vi.fn(), + getInput: vi.fn(), + getBooleanInput: vi.fn(), + getMultilineInput: vi.fn(), + getState: vi.fn(), + saveState: vi.fn(), + startGroup: vi.fn(), + endGroup: vi.fn(), + group: vi.fn(), + addPath: vi.fn(), + setCommandEcho: vi.fn(), + isDebug: vi.fn().mockReturnValue(false), + getIDToken: vi.fn(), + toPlatformPath: vi.fn(), + toPosixPath: vi.fn(), + toWin32Path: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn().mockResolvedValue(), + }, +}; + +const mockGithub = { + rest: { + pulls: { + createReviewComment: vi.fn(), + get: vi.fn(), + }, + }, +}; + +const mockContext = { + eventName: "pull_request", + runId: 12345, + repo: { owner: "testowner", repo: "testrepo" }, + payload: { + pull_request: { number: 123, head: { sha: "abc123def456" } }, + repository: { + html_url: "https://github.com/testowner/testrepo", + }, }, - mockGithub = { rest: { pulls: { createReviewComment: vi.fn() } } }, - mockContext = { - eventName: "pull_request", - runId: 12345, - repo: { owner: "testowner", repo: "testrepo" }, - payload: { pull_request: { number: 123, head: { sha: "abc123def456" } }, repository: { html_url: "https://github.com/testowner/testrepo" } }, - }; -((global.core = mockCore), - (global.github = mockGithub), - (global.context = mockContext), - describe("create_pr_review_comment.cjs", () => { - let createPRReviewCommentScript, tempFilePath; - const setAgentOutput = data => { - tempFilePath = path.join("/tmp", `test_agent_output_${Date.now()}_${Math.random().toString(36).slice(2)}.json`); - const content = "string" == typeof data ? data : JSON.stringify(data); - (fs.writeFileSync(tempFilePath, content), (process.env.GH_AW_AGENT_OUTPUT = tempFilePath)); +}; + +global.core = mockCore; +global.github = mockGithub; +global.context = mockContext; + +const { createReviewBuffer } = require("./pr_review_buffer.cjs"); + +describe("create_pr_review_comment.cjs", () => { + let createPRReviewCommentScript; + let buffer; + + beforeEach(() => { + vi.clearAllMocks(); + + // Create a fresh buffer for each test (factory pattern, no global state) + buffer = createReviewBuffer(); + + const scriptPath = path.join(__dirname, "create_pr_review_comment.cjs"); + createPRReviewCommentScript = fs.readFileSync(scriptPath, "utf8"); + + delete process.env.GH_AW_AGENT_OUTPUT; + delete process.env.GH_AW_PR_REVIEW_COMMENT_SIDE; + delete process.env.GH_AW_PR_REVIEW_COMMENT_TARGET; + delete process.env.GH_AW_WORKFLOW_NAME; + delete process.env.GH_AW_WORKFLOW_SOURCE; + delete process.env.GH_AW_WORKFLOW_SOURCE_URL; + delete process.env.GH_AW_PROMPTS_DIR; + + global.context = { + eventName: "pull_request", + runId: 12345, + repo: { owner: "testowner", repo: "testrepo" }, + payload: { + pull_request: { number: 123, head: { sha: "abc123def456" } }, + repository: { + html_url: "https://github.com/testowner/testrepo", + }, + }, + }; + }); + + /** Helper to create a handler with the buffer injected via config */ + async function createHandler(extraConfig = {}) { + // buffer is a local var accessible in eval scope; JSON.stringify can't serialize it (has functions) + const configStr = JSON.stringify(extraConfig); + return await eval(`(async () => { ${createPRReviewCommentScript}; return await main(Object.assign({ _prReviewBuffer: buffer }, ${configStr})); })()`); + } + + it("should return error when no buffer provided", async () => { + const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); + const result = await handler({}, {}); + expect(result.success).toBe(false); + expect(result.error).toContain("No PR review buffer available"); + }); + + it("should buffer a single PR review comment with basic configuration", async () => { + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Consider using const instead of let here.", + }; + const handler = await createHandler(); + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(result.pull_request_number).toBe(123); + expect(buffer.getBufferedCount()).toBe(1); + // Individual comments should NOT be posted directly + expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); + }); + + it("should buffer a multi-line PR review comment", async () => { + const message = { + type: "create_pull_request_review_comment", + path: "src/utils.js", + line: 25, + start_line: 20, + side: "LEFT", + body: "This entire function could be simplified using modern JS features.", + }; + const handler = await createHandler(); + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(buffer.getBufferedCount()).toBe(1); + expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); + }); + + it("should buffer multiple review comments", async () => { + const handler = await createHandler(); + const message1 = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "First comment", + }; + const message2 = { + type: "create_pull_request_review_comment", + path: "src/utils.js", + line: 25, + body: "Second comment", + }; + const result1 = await handler(message1, {}); + const result2 = await handler(message2, {}); + + expect(result1.success).toBe(true); + expect(result2.success).toBe(true); + expect(buffer.getBufferedCount()).toBe(2); + expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); + }); + + it("should enforce max count limit", async () => { + const handler = await createHandler({ max: 2 }); + const message1 = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "First comment", + }; + const message2 = { + type: "create_pull_request_review_comment", + path: "src/utils.js", + line: 20, + body: "Second comment", + }; + const message3 = { + type: "create_pull_request_review_comment", + path: "src/test.js", + line: 30, + body: "Third comment", + }; + + const result1 = await handler(message1, {}); + const result2 = await handler(message2, {}); + const result3 = await handler(message3, {}); + + expect(result1.success).toBe(true); + expect(result2.success).toBe(true); + expect(result3.success).toBe(false); + expect(result3.error).toContain("Max count of 2 reached"); + expect(buffer.getBufferedCount()).toBe(2); + }); + + it("should use configured side from config", async () => { + const handler = await createHandler({ side: "LEFT" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Comment on left side", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(buffer.getBufferedCount()).toBe(1); + }); + + it("should skip when not in pull request context", async () => { + global.context = { + ...mockContext, + eventName: "issues", + payload: { + issue: { number: 123 }, + repository: mockContext.payload.repository, + }, + }; + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "This should not be created", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Not in pull request context"); + expect(buffer.getBufferedCount()).toBe(0); + }); + + it("should validate required fields and skip invalid items", async () => { + const handler = await createHandler(); + + // Missing path + const result1 = await handler({ type: "create_pull_request_review_comment", line: 10, body: "Missing path" }, {}); + expect(result1.success).toBe(false); + expect(result1.error).toContain('Missing required field "path"'); + + // Missing line + const result2 = await handler( + { + type: "create_pull_request_review_comment", + path: "src/main.js", + body: "Missing line", + }, + {} + ); + expect(result2.success).toBe(false); + expect(result2.error).toContain('Missing or invalid required field "line"'); + + // Missing body + const result3 = await handler( + { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + }, + {} + ); + expect(result3.success).toBe(false); + expect(result3.error).toContain('Missing or invalid required field "body"'); + + expect(buffer.getBufferedCount()).toBe(0); + }); + + it("should validate start_line is not greater than line", async () => { + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + start_line: 15, + body: "Invalid range", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Invalid start_line"); + expect(buffer.getBufferedCount()).toBe(0); + }); + + it("should validate side values", async () => { + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + side: "INVALID_SIDE", + body: "Invalid side value", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Invalid side value"); + expect(buffer.getBufferedCount()).toBe(0); + }); + + it("should buffer comment body without footer (footer is added at review level)", async () => { + process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Original comment", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(buffer.getBufferedCount()).toBe(1); + // Footer is added at review submission time, not per-comment + expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); + }); + + it("should respect target configuration for specific PR number", async () => { + mockGithub.rest.pulls.get.mockResolvedValue({ + data: { number: 456, head: { sha: "def456abc789" } }, + }); + const handler = await createHandler({ target: "456" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Review comment on specific PR", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(result.pull_request_number).toBe(456); + expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 456, + }); + expect(buffer.getBufferedCount()).toBe(1); + }); + + it('should respect target "*" configuration with pull_request_number in item', async () => { + mockGithub.rest.pulls.get.mockResolvedValue({ + data: { number: 789, head: { sha: "xyz789abc456" } }, + }); + const handler = await createHandler({ target: "*" }); + const message = { + type: "create_pull_request_review_comment", + pull_request_number: 789, + path: "src/utils.js", + line: 20, + body: "Review comment on any PR", }; - (beforeEach(() => { - vi.clearAllMocks(); - const scriptPath = path.join(__dirname, "create_pr_review_comment.cjs"); - ((createPRReviewCommentScript = fs.readFileSync(scriptPath, "utf8")), - delete process.env.GH_AW_AGENT_OUTPUT, - delete process.env.GH_AW_PR_REVIEW_COMMENT_SIDE, - delete process.env.GH_AW_PR_REVIEW_COMMENT_TARGET, - (global.context = mockContext)); - }), - afterEach(() => { - tempFilePath && require("fs").existsSync(tempFilePath) && (require("fs").unlinkSync(tempFilePath), (tempFilePath = void 0)); - }), - it("should create a single PR review comment with basic configuration", async () => { - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: { id: 456, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r456" } }); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Consider using const instead of let here." }; - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledWith({ - owner: "testowner", - repo: "testrepo", - pull_number: 123, - body: expect.stringContaining("Consider using const instead of let here."), - path: "src/main.js", - commit_id: "abc123def456", - line: 10, - side: "RIGHT", - }); - }), - it("should create a multi-line PR review comment", async () => { - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: { id: 789, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r789" } }); - const message = { type: "create_pull_request_review_comment", path: "src/utils.js", line: 25, start_line: 20, side: "LEFT", body: "This entire function could be simplified using modern JS features." }; - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledWith({ - owner: "testowner", - repo: "testrepo", - pull_number: 123, - body: expect.stringContaining("This entire function could be simplified using modern JS features."), - path: "src/utils.js", - commit_id: "abc123def456", - line: 25, - start_line: 20, - side: "LEFT", - start_side: "LEFT", - }); - }), - it("should handle multiple review comments", async () => { - mockGithub.rest.pulls.createReviewComment - .mockResolvedValueOnce({ data: { id: 111, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r111" } }) - .mockResolvedValueOnce({ data: { id: 222, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r222" } }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message1 = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "First comment" }; - const message2 = { type: "create_pull_request_review_comment", path: "src/utils.js", line: 25, body: "Second comment" }; - const result1 = await handler(message1, {}); - const result2 = await handler(message2, {}); - expect(result1.success).toBe(true); - expect(result2.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledTimes(2); - }), - it("should enforce max count limit", async () => { - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: { id: 123, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r123" } }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({ max: 2 }); })()`); - const message1 = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "First comment" }; - const message2 = { type: "create_pull_request_review_comment", path: "src/utils.js", line: 20, body: "Second comment" }; - const message3 = { type: "create_pull_request_review_comment", path: "src/test.js", line: 30, body: "Third comment" }; - - const result1 = await handler(message1, {}); - const result2 = await handler(message2, {}); - const result3 = await handler(message3, {}); - - expect(result1.success).toBe(true); - expect(result2.success).toBe(true); - expect(result3.success).toBe(false); - expect(result3.error).toContain("Max count of 2 reached"); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledTimes(2); - }), - it("should use configured side from config", async () => { - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: { id: 333, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r333" } }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({ side: 'LEFT' }); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Comment on left side" }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledWith(expect.objectContaining({ side: "LEFT" })); - }), - it("should skip when not in pull request context", async () => { - global.context = { ...mockContext, eventName: "issues", payload: { issue: { number: 123 }, repository: mockContext.payload.repository } }; - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "This should not be created" }; - const result = await handler(message, {}); - expect(result.success).toBe(false); - expect(result.error).toContain("Not in pull request context"); - expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); - }), - it("should validate required fields and skip invalid items", async () => { - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - - // Missing path - const result1 = await handler({ type: "create_pull_request_review_comment", line: 10, body: "Missing path" }, {}); - expect(result1.success).toBe(false); - expect(result1.error).toContain('Missing required field "path"'); - - // Missing line - const result2 = await handler({ type: "create_pull_request_review_comment", path: "src/main.js", body: "Missing line" }, {}); - expect(result2.success).toBe(false); - expect(result2.error).toContain('Missing or invalid required field "line"'); - - // Missing body - const result3 = await handler({ type: "create_pull_request_review_comment", path: "src/main.js", line: 10 }, {}); - expect(result3.success).toBe(false); - expect(result3.error).toContain('Missing or invalid required field "body"'); - - expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); - }), - it("should validate start_line is not greater than line", async () => { - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, start_line: 15, body: "Invalid range" }; - const result = await handler(message, {}); - expect(result.success).toBe(false); - expect(result.error).toContain("Invalid start_line"); - expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); - }), - it("should validate side values", async () => { - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, side: "INVALID_SIDE", body: "Invalid side value" }; - const result = await handler(message, {}); - expect(result.success).toBe(false); - expect(result.error).toContain("Invalid side value"); - expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); - }), - it("should include AI disclaimer in comment body", async () => { - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: { id: 999, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r999" } }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Original comment" }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledWith(expect.objectContaining({ body: expect.stringMatching(/Original comment[\s\S]*AI generated by/) })); - }), - it("should respect target configuration for specific PR number", async () => { - mockGithub.rest.pulls.get = vi.fn().mockResolvedValue({ data: { number: 456, head: { sha: "def456abc789" } } }); - mockGithub.rest.pulls.createReviewComment = vi.fn().mockResolvedValue({ data: { id: 999, html_url: "https://github.com/testowner/testrepo/pull/456#discussion_r999" } }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({ target: '456' }); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Review comment on specific PR" }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith({ owner: "testowner", repo: "testrepo", pull_number: 456 }); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 456, path: "src/main.js", line: 10, commit_id: "def456abc789" })); - }), - it('should respect target "*" configuration with pull_request_number in item', async () => { - mockGithub.rest.pulls.get = vi.fn().mockResolvedValue({ data: { number: 789, head: { sha: "xyz789abc456" } } }); - mockGithub.rest.pulls.createReviewComment = vi.fn().mockResolvedValue({ data: { id: 888, html_url: "https://github.com/testowner/testrepo/pull/789#discussion_r888" } }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({ target: '*' }); })()`); - const message = { type: "create_pull_request_review_comment", pull_request_number: 789, path: "src/utils.js", line: 20, body: "Review comment on any PR" }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith({ owner: "testowner", repo: "testrepo", pull_number: 789 }); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 789, path: "src/utils.js", line: 20, commit_id: "xyz789abc456" })); - }), - it('should skip item when target is "*" but no pull_request_number specified', async () => { - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({ target: '*' }); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Review comment without PR number" }; - const result = await handler(message, {}); - expect(result.success).toBe(false); - expect(result.error).toContain('Target is "*" but no pull_request_number specified'); - expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); - }), - it("should skip comment creation when target is triggering but not in PR context", async () => { - global.context = { eventName: "issues", runId: 12345, repo: { owner: "testowner", repo: "testrepo" }, payload: { issue: { number: 10 }, repository: { html_url: "https://github.com/testowner/testrepo" } } }; - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({ target: 'triggering' }); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "This should not be created" }; - const result = await handler(message, {}); - expect(result.success).toBe(false); - expect(result.error).toContain("Not in pull request context"); - expect(mockGithub.rest.pulls.createReviewComment).not.toHaveBeenCalled(); - }), - it("should include workflow source in footer when GH_AW_WORKFLOW_SOURCE is provided", async () => { - process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; - process.env.GH_AW_WORKFLOW_SOURCE = "githubnext/agentics/workflows/ci-doctor.md@v1.0.0"; - process.env.GH_AW_WORKFLOW_SOURCE_URL = "https://github.com/githubnext/agentics/tree/v1.0.0/workflows/ci-doctor.md"; - process.env.GH_AW_PROMPTS_DIR = path.join(__dirname, "..", "md"); - global.context = { - eventName: "pull_request", - runId: 12345, - repo: { owner: "testowner", repo: "testrepo" }, - payload: { pull_request: { number: 10, head: { sha: "abc123" } }, repository: { html_url: "https://github.com/testowner/testrepo" } }, - }; - const mockComment = { id: 456, html_url: "https://github.com/testowner/testrepo/pull/10#discussion_r456" }; - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: mockComment }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Test review comment with source" }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalled(); - const callArgs = mockGithub.rest.pulls.createReviewComment.mock.calls[0][0]; - expect(callArgs.body).toContain("Test review comment with source"); - expect(callArgs.body).toContain("AI generated by [Test Workflow]"); - expect(callArgs.body).toContain("https://github.com/testowner/testrepo/actions/runs/12345"); - expect(callArgs.body).toContain("gh aw add githubnext/agentics/workflows/ci-doctor.md@v1.0.0"); - expect(callArgs.body).toContain("usage guide"); - }), - it("should not include workflow source footer when GH_AW_WORKFLOW_SOURCE is not provided", async () => { - process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; - delete process.env.GH_AW_WORKFLOW_SOURCE; - global.context = { - eventName: "pull_request", - runId: 12345, - repo: { owner: "testowner", repo: "testrepo" }, - payload: { pull_request: { number: 10, head: { sha: "abc123" } }, repository: { html_url: "https://github.com/testowner/testrepo" } }, - }; - const mockComment = { id: 457, html_url: "https://github.com/testowner/testrepo/pull/10#discussion_r457" }; - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: mockComment }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", path: "src/main.js", line: 10, body: "Test review comment without source" }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - expect(mockGithub.rest.pulls.createReviewComment).toHaveBeenCalled(); - const callArgs = mockGithub.rest.pulls.createReviewComment.mock.calls[0][0]; - expect(callArgs.body).toContain("Test review comment without source"); - expect(callArgs.body).toContain("AI generated by [Test Workflow]"); - expect(callArgs.body).not.toContain("gh aw add"); - expect(callArgs.body).not.toContain("usage guide"); - }), - it("should include triggering PR number in footer when in PR context", async () => { - process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; - global.context.eventName = "pull_request"; - global.context.payload.pull_request = { number: 123, head: { sha: "abc123" } }; - const mockComment = { id: 999, html_url: "https://github.com/testowner/testrepo/pull/123#discussion_r999" }; - mockGithub.rest.pulls.createReviewComment.mockResolvedValue({ data: mockComment }); - const handler = await eval(`(async () => { ${createPRReviewCommentScript}; return await main({}); })()`); - const message = { type: "create_pull_request_review_comment", body: "Review comment from PR context", path: "test.js", line: 10 }; - const result = await handler(message, {}); - expect(result.success).toBe(true); - const callArgs = mockGithub.rest.pulls.createReviewComment.mock.calls[0][0]; - expect(callArgs.body).toContain("Review comment from PR context"); - expect(callArgs.body).toContain("AI generated by [Test Workflow]"); - expect(callArgs.body).toContain("for #123"); - })); - })); + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + expect(result.pull_request_number).toBe(789); + expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith({ + owner: "testowner", + repo: "testrepo", + pull_number: 789, + }); + expect(buffer.getBufferedCount()).toBe(1); + }); + + it('should skip item when target is "*" but no pull_request_number specified', async () => { + const handler = await createHandler({ target: "*" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Review comment without PR number", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain('Target is "*" but no pull_request_number specified'); + expect(buffer.getBufferedCount()).toBe(0); + }); + + it("should skip comment creation when target is triggering but not in PR context", async () => { + global.context = { + eventName: "issues", + runId: 12345, + repo: { owner: "testowner", repo: "testrepo" }, + payload: { + issue: { number: 10 }, + repository: { + html_url: "https://github.com/testowner/testrepo", + }, + }, + }; + const handler = await createHandler({ target: "triggering" }); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "This should not be created", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Not in pull request context"); + expect(buffer.getBufferedCount()).toBe(0); + }); + + it("should reject comments targeting a different PR than the first comment", async () => { + // First comment sets context to PR #123 + const handler = await createHandler(); + const message1 = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "First comment on PR 123", + }; + const result1 = await handler(message1, {}); + expect(result1.success).toBe(true); + expect(buffer.getBufferedCount()).toBe(1); + + // Simulate a second comment targeting a different PR by changing context + global.context = { + eventName: "pull_request", + runId: 12345, + repo: { owner: "testowner", repo: "testrepo" }, + payload: { + pull_request: { number: 456, head: { sha: "def456" } }, + repository: { + html_url: "https://github.com/testowner/testrepo", + }, + }, + }; + + // Need a new handler that sees the different context but shares the same buffer + const handler2 = await createHandler(); + const message2 = { + type: "create_pull_request_review_comment", + path: "src/other.js", + line: 20, + body: "Comment on different PR", + }; + const result2 = await handler2(message2, {}); + + expect(result2.success).toBe(false); + expect(result2.error).toContain("must target the same PR"); + expect(buffer.getBufferedCount()).toBe(1); // Still only 1 comment + }); + + it("should set footer context for review-level footer generation", async () => { + process.env.GH_AW_WORKFLOW_NAME = "Test Workflow"; + process.env.GH_AW_WORKFLOW_SOURCE = "githubnext/agentics/workflows/ci-doctor.md@v1.0.0"; + process.env.GH_AW_WORKFLOW_SOURCE_URL = "https://github.com/githubnext/agentics/tree/v1.0.0/workflows/ci-doctor.md"; + + const handler = await createHandler(); + const message = { + type: "create_pull_request_review_comment", + path: "src/main.js", + line: 10, + body: "Test review comment", + }; + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.buffered).toBe(true); + // Footer context is set on the buffer for review-level footer generation + expect(buffer.getBufferedCount()).toBe(1); + }); +}); diff --git a/actions/setup/js/package-lock.json b/actions/setup/js/package-lock.json index dee34bbdd8..e3cdbb8e50 100644 --- a/actions/setup/js/package-lock.json +++ b/actions/setup/js/package-lock.json @@ -939,7 +939,6 @@ "integrity": "sha512-/g2d4sW9nUDJOMz3mabVQvOGhVa4e/BN/Um7yca9Bb2XTzPPnfTWHWQg+IsEYO7M3Vx+EXvaM/I2pJWIMun1bg==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@octokit/auth-token": "^4.0.0", "@octokit/graphql": "^7.1.0", @@ -1480,7 +1479,6 @@ "integrity": "sha512-m0jEgYlYz+mDJZ2+F4v8D1AyQb+QzsNqRuI7xg1VQX/KlKS0qT9r1Mo16yo5F/MtifXFgaofIFsdFMox2SxIbQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "undici-types": "~7.16.0" } @@ -1619,7 +1617,6 @@ "integrity": "sha512-CGJ25bc8fRi8Lod/3GHSvXRKi7nBo3kxh0ApW4yCjmrWmRmlT53B5E08XRSZRliygG0aVNxLrBEqPYdz/KcCtQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/utils": "4.0.18", "fflate": "^0.8.2", @@ -2021,7 +2018,6 @@ "integrity": "sha512-5gTmgEY/sqK6gFXLIsQNH19lWb4ebPDLA4SdLP7dsWkIXHWlG66oPuVvXSGFPppYZz8ZDZq0dYYrbHfBCVUb1Q==", "dev": true, "license": "MIT", - "peer": true, "engines": { "node": ">=12" }, @@ -2296,7 +2292,6 @@ "integrity": "sha512-w+N7Hifpc3gRjZ63vYBXA56dvvRlNWRczTdmCBBa+CotUzAPf5b7YMdMR/8CQoeYE5LX3W4wj6RYTgonm1b9DA==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "esbuild": "^0.27.0", "fdir": "^6.5.0", @@ -2372,7 +2367,6 @@ "integrity": "sha512-hOQuK7h0FGKgBAas7v0mSAsnvrIgAvWmRFjmzpJ7SwFHH3g1k2u37JtYwOwmEKhK6ZO3v9ggDBBm0La1LCK4uQ==", "dev": true, "license": "MIT", - "peer": true, "dependencies": { "@vitest/expect": "4.0.18", "@vitest/mocker": "4.0.18", diff --git a/actions/setup/js/pr_review_buffer.cjs b/actions/setup/js/pr_review_buffer.cjs new file mode 100644 index 0000000000..6ebb3a3293 --- /dev/null +++ b/actions/setup/js/pr_review_buffer.cjs @@ -0,0 +1,280 @@ +// @ts-check +/// + +/** + * PR Review Buffer Factory + * + * Creates a buffer instance that collects PR review comments and review metadata + * so they can be submitted as a single GitHub PR review via pulls.createReview(). + * + * Usage: + * const { createReviewBuffer } = require("./pr_review_buffer.cjs"); + * const buffer = createReviewBuffer(); + * buffer.addComment({ path: "file.js", line: 10, body: "Fix this" }); + * buffer.setReviewMetadata("LGTM", "APPROVE"); + * await buffer.submitReview(); + */ + +const { generateFooter } = require("./generate_footer.cjs"); +const { getErrorMessage } = require("./error_helpers.cjs"); + +/** + * @typedef {Object} BufferedComment + * @property {string} path - File path relative to repo root + * @property {number} line - Line number (end line for multi-line) + * @property {string} body - Comment body text + * @property {number} [start_line] - Start line for multi-line comments + * @property {string} [side] - LEFT or RIGHT + * @property {string} [start_side] - start_side for multi-line comments + */ + +/** + * @typedef {Object} ReviewMetadata + * @property {string} body - Overall review body text + * @property {string} event - Review event: APPROVE, REQUEST_CHANGES, or COMMENT + */ + +/** + * @typedef {Object} ReviewContext + * @property {string} repo - Repository slug (owner/repo) + * @property {{owner: string, repo: string}} repoParts - Parsed owner and repo + * @property {number} pullRequestNumber - PR number + * @property {Object} pullRequest - Full PR object with head.sha + */ + +/** + * Create a new PR review buffer instance. + * All state is encapsulated in the returned object — no module-level globals. + * + * @returns {Object} Buffer instance with methods to add comments, set metadata, and submit review + */ +function createReviewBuffer() { + /** @type {BufferedComment[]} */ + const bufferedComments = []; + + /** @type {ReviewMetadata | null} */ + let reviewMetadata = null; + + /** @type {ReviewContext | null} */ + let reviewContext = null; + + /** @type {{workflowName: string, runUrl: string, workflowSource: string, workflowSourceURL: string, triggeringIssueNumber: number|undefined, triggeringPRNumber: number|undefined, triggeringDiscussionNumber: number|undefined} | null} */ + let footerContext = null; + + /** + * Add a validated comment to the buffer. + * Rejects comments targeting a different repo/PR than the first comment. + * @param {BufferedComment} comment - Validated comment to buffer + */ + function addComment(comment) { + bufferedComments.push(comment); + core.info(`Buffered review comment ${bufferedComments.length}: ${comment.path}:${comment.line}`); + } + + /** + * Set the review metadata (body and event). + * Overwrites any previously set metadata (last call wins). + * @param {string} body - Overall review body text + * @param {string} event - Review event: APPROVE, REQUEST_CHANGES, or COMMENT + */ + function setReviewMetadata(body, event) { + reviewMetadata = { body, event }; + core.info(`Set review metadata: event=${event}, bodyLength=${body.length}`); + } + + /** + * Set the review context (target repo and PR). + * Only sets if not already set (first comment determines context). + * @param {ReviewContext} ctx - Review context + * @returns {boolean} true if context was set, false if already set + */ + function setReviewContext(ctx) { + if (reviewContext === null) { + reviewContext = ctx; + core.info(`Set review context: ${ctx.repo}#${ctx.pullRequestNumber}`); + return true; + } + return false; + } + + /** + * Get the current review context (repo and PR). + * @returns {ReviewContext | null} + */ + function getReviewContext() { + return reviewContext; + } + + /** + * Set the footer context for generating review footer. + * Only sets if not already set. + * @param {Object} ctx - Footer context + */ + function setFooterContext(ctx) { + if (footerContext === null) { + footerContext = ctx; + } + } + + /** + * Check if there are buffered comments to submit. + * @returns {boolean} + */ + function hasBufferedComments() { + return bufferedComments.length > 0; + } + + /** + * Check if review metadata has been set. + * @returns {boolean} + */ + function hasReviewMetadata() { + return reviewMetadata !== null; + } + + /** + * Get the number of buffered comments. + * @returns {number} + */ + function getBufferedCount() { + return bufferedComments.length; + } + + /** + * Submit the buffered review as a single pulls.createReview() call. + * Supports body-only reviews (no inline comments) when metadata is set. + * If no submit_pull_request_review message was provided, defaults to event: "COMMENT". + * + * @returns {Promise} Result with success status and review details + */ + async function submitReview() { + if (bufferedComments.length === 0 && !reviewMetadata) { + core.info("No buffered review comments or review metadata to submit"); + return { success: true, skipped: true }; + } + + if (!reviewContext) { + core.warning("No review context set - cannot submit review"); + return { success: false, error: "No review context available" }; + } + + const { repo, repoParts, pullRequestNumber, pullRequest } = reviewContext; + + if (!pullRequest || !pullRequest.head || !pullRequest.head.sha) { + core.warning("Pull request head SHA not available - cannot submit review"); + return { success: false, error: "Pull request head SHA not available" }; + } + + // Determine review event and body + const event = reviewMetadata ? reviewMetadata.event : "COMMENT"; + let body = reviewMetadata ? reviewMetadata.body : ""; + + // Add footer to review body if we have footer context + if (footerContext) { + body += generateFooter( + footerContext.workflowName, + footerContext.runUrl, + footerContext.workflowSource, + footerContext.workflowSourceURL, + footerContext.triggeringIssueNumber, + footerContext.triggeringPRNumber, + footerContext.triggeringDiscussionNumber + ); + } + + // Build comments array for the API + const comments = bufferedComments.map(comment => { + /** @type {any} */ + const apiComment = { + path: comment.path, + line: comment.line, + body: comment.body, + }; + + if (comment.start_line !== undefined) { + apiComment.start_line = comment.start_line; + } + + if (comment.side) { + apiComment.side = comment.side; + } + + if (comment.start_line !== undefined && comment.start_side) { + apiComment.start_side = comment.start_side; + } else if (comment.start_line !== undefined && comment.side) { + // Fall back to side when start_side is not explicitly provided + apiComment.start_side = comment.side; + } + + return apiComment; + }); + + core.info(`Submitting PR review on ${repo}#${pullRequestNumber}: event=${event}, comments=${comments.length}, bodyLength=${body.length}`); + + try { + /** @type {any} */ + const requestParams = { + owner: repoParts.owner, + repo: repoParts.repo, + pull_number: pullRequestNumber, + commit_id: pullRequest.head.sha, + event: event, + }; + + // Only include comments if there are any + if (comments.length > 0) { + requestParams.comments = comments; + } + + // Only include body if non-empty (body is required for APPROVE and REQUEST_CHANGES) + if (body) { + requestParams.body = body; + } + + const { data: review } = await github.rest.pulls.createReview(requestParams); + + core.info(`Created PR review #${review.id}: ${review.html_url}`); + + return { + success: true, + review_id: review.id, + review_url: review.html_url, + pull_request_number: pullRequestNumber, + repo: repo, + event: event, + comment_count: comments.length, + }; + } catch (error) { + core.error(`Failed to submit PR review: ${getErrorMessage(error)}`); + return { + success: false, + error: getErrorMessage(error), + }; + } + } + + /** + * Reset the buffer state (for testing). + */ + function reset() { + bufferedComments.length = 0; + reviewMetadata = null; + reviewContext = null; + footerContext = null; + } + + return { + addComment, + setReviewMetadata, + setReviewContext, + getReviewContext, + setFooterContext, + hasBufferedComments, + hasReviewMetadata, + getBufferedCount, + submitReview, + reset, + }; +} + +module.exports = { createReviewBuffer }; diff --git a/actions/setup/js/pr_review_buffer.test.cjs b/actions/setup/js/pr_review_buffer.test.cjs new file mode 100644 index 0000000000..5d66f530d8 --- /dev/null +++ b/actions/setup/js/pr_review_buffer.test.cjs @@ -0,0 +1,476 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import fs from "fs"; +import os from "os"; +import path from "path"; + +const mockCore = { + debug: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), +}; + +const mockGithub = { + rest: { + pulls: { + createReview: vi.fn(), + }, + }, +}; + +global.core = mockCore; +global.github = mockGithub; + +const { createReviewBuffer } = require("./pr_review_buffer.cjs"); + +describe("pr_review_buffer (factory pattern)", () => { + let buffer; + let testPromptsDir; + let originalEnv; + + beforeEach(() => { + vi.clearAllMocks(); + + // Save and set up test prompts directory for generateFooter + originalEnv = process.env.GH_AW_PROMPTS_DIR; + testPromptsDir = path.join(os.tmpdir(), "gh-aw-test-pr-review-buffer", "prompts"); + fs.mkdirSync(testPromptsDir, { recursive: true }); + const templateContent = "Install this workflow: `gh aw add {workflow_source}` ([usage guide](https://github.com/github/gh-aw))"; + fs.writeFileSync(path.join(testPromptsDir, "workflow_install_note.md"), templateContent, "utf8"); + process.env.GH_AW_PROMPTS_DIR = testPromptsDir; + + // Create a fresh buffer instance for each test (no shared global state) + buffer = createReviewBuffer(); + }); + + afterEach(() => { + // Restore original environment + if (originalEnv !== undefined) { + process.env.GH_AW_PROMPTS_DIR = originalEnv; + } else { + delete process.env.GH_AW_PROMPTS_DIR; + } + + // Clean up test directory + try { + fs.rmSync(path.dirname(testPromptsDir), { recursive: true, force: true }); + } catch { + // Ignore cleanup errors + } + }); + + it("should create independent buffer instances", () => { + const buffer1 = createReviewBuffer(); + const buffer2 = createReviewBuffer(); + + buffer1.addComment({ path: "file.js", line: 1, body: "comment" }); + + expect(buffer1.getBufferedCount()).toBe(1); + expect(buffer2.getBufferedCount()).toBe(0); + }); + + describe("addComment", () => { + it("should buffer a single comment", () => { + buffer.addComment({ path: "src/index.js", line: 10, body: "Fix this" }); + + expect(buffer.hasBufferedComments()).toBe(true); + expect(buffer.getBufferedCount()).toBe(1); + }); + + it("should buffer multiple comments", () => { + buffer.addComment({ path: "src/index.js", line: 10, body: "Fix this" }); + buffer.addComment({ path: "src/utils.js", line: 20, body: "And this" }); + buffer.addComment({ + path: "src/app.js", + line: 30, + body: "Also this", + start_line: 25, + }); + + expect(buffer.getBufferedCount()).toBe(3); + }); + }); + + describe("setReviewMetadata", () => { + it("should store review body and event", () => { + buffer.setReviewMetadata("Great changes!", "APPROVE"); + + expect(buffer.hasReviewMetadata()).toBe(true); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("event=APPROVE")); + }); + }); + + describe("setReviewContext", () => { + it("should set context on first call and return true", () => { + const ctx = { + repo: "test-owner/test-repo", + repoParts: { owner: "test-owner", repo: "test-repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }; + + const result = buffer.setReviewContext(ctx); + + expect(result).toBe(true); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("test-owner/test-repo#42")); + }); + + it("should not override context on subsequent calls and return false", () => { + const ctx1 = { + repo: "owner/repo1", + repoParts: { owner: "owner", repo: "repo1" }, + pullRequestNumber: 1, + pullRequest: { head: { sha: "sha1" } }, + }; + + const ctx2 = { + repo: "owner/repo2", + repoParts: { owner: "owner", repo: "repo2" }, + pullRequestNumber: 2, + pullRequest: { head: { sha: "sha2" } }, + }; + + const result1 = buffer.setReviewContext(ctx1); + const result2 = buffer.setReviewContext(ctx2); + + expect(result1).toBe(true); + expect(result2).toBe(false); + expect(mockCore.info).toHaveBeenCalledWith(expect.stringContaining("owner/repo1#1")); + expect(mockCore.info).not.toHaveBeenCalledWith(expect.stringContaining("owner/repo2#2")); + }); + }); + + describe("getReviewContext", () => { + it("should return null when no context set", () => { + expect(buffer.getReviewContext()).toBeNull(); + }); + + it("should return the set context", () => { + const ctx = { + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 1, + pullRequest: { head: { sha: "abc" } }, + }; + buffer.setReviewContext(ctx); + expect(buffer.getReviewContext()).toEqual(ctx); + }); + }); + + describe("hasBufferedComments / getBufferedCount / hasReviewMetadata", () => { + it("should return false/0 when empty", () => { + expect(buffer.hasBufferedComments()).toBe(false); + expect(buffer.getBufferedCount()).toBe(0); + expect(buffer.hasReviewMetadata()).toBe(false); + }); + + it("should return true/count after adding comments", () => { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + + expect(buffer.hasBufferedComments()).toBe(true); + expect(buffer.getBufferedCount()).toBe(1); + }); + + it("should report metadata after setting it", () => { + buffer.setReviewMetadata("body", "APPROVE"); + expect(buffer.hasReviewMetadata()).toBe(true); + }); + }); + + describe("submitReview", () => { + it("should skip when no comments and no metadata are present", async () => { + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(result.skipped).toBe(true); + expect(mockGithub.rest.pulls.createReview).not.toHaveBeenCalled(); + }); + + it("should fail when no review context is set", async () => { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(false); + expect(result.error).toContain("No review context available"); + }); + + it("should fail when PR head SHA is missing", async () => { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 1, + pullRequest: {}, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(false); + expect(result.error).toContain("head SHA not available"); + }); + + it("should submit review with default COMMENT event when no metadata set", async () => { + buffer.addComment({ path: "src/index.js", line: 10, body: "Fix this" }); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 100, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-100", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(result.event).toBe("COMMENT"); + expect(result.comment_count).toBe(1); + expect(mockGithub.rest.pulls.createReview).toHaveBeenCalledWith({ + owner: "owner", + repo: "repo", + pull_number: 42, + commit_id: "abc123", + event: "COMMENT", + comments: [{ path: "src/index.js", line: 10, body: "Fix this" }], + }); + }); + + it("should submit review with metadata when set", async () => { + buffer.addComment({ path: "src/index.js", line: 10, body: "Fix this" }); + buffer.setReviewMetadata("Please address these issues.", "REQUEST_CHANGES"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 200, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-200", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(result.event).toBe("REQUEST_CHANGES"); + expect(result.review_id).toBe(200); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.event).toBe("REQUEST_CHANGES"); + expect(callArgs.body).toContain("Please address these issues."); + }); + + it("should submit body-only review when metadata set but no comments", async () => { + buffer.setReviewMetadata("LGTM! Approving this change.", "APPROVE"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 600, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-600", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(result.event).toBe("APPROVE"); + expect(result.comment_count).toBe(0); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.event).toBe("APPROVE"); + expect(callArgs.body).toContain("LGTM! Approving this change."); + // No comments array when empty + expect(callArgs.comments).toBeUndefined(); + }); + + it("should include multi-line comment fields with side fallback for start_side", async () => { + buffer.addComment({ + path: "src/index.js", + line: 15, + body: "Multi-line issue", + start_line: 10, + side: "RIGHT", + }); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 300, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-300", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + // When start_side is not explicitly set, falls back to side + expect(callArgs.comments[0]).toEqual({ + path: "src/index.js", + line: 15, + body: "Multi-line issue", + start_line: 10, + side: "RIGHT", + start_side: "RIGHT", + }); + }); + + it("should use explicit start_side when provided", async () => { + buffer.addComment({ + path: "src/index.js", + line: 15, + body: "Cross-side comment", + start_line: 10, + side: "RIGHT", + start_side: "LEFT", + }); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 300, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-300", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.comments[0]).toEqual({ + path: "src/index.js", + line: 15, + body: "Cross-side comment", + start_line: 10, + side: "RIGHT", + start_side: "LEFT", + }); + }); + + it("should append footer when footer context is set", async () => { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + buffer.setReviewMetadata("Review body", "COMMENT"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + buffer.setFooterContext({ + workflowName: "test-workflow", + runUrl: "https://github.com/owner/repo/actions/runs/123", + workflowSource: "owner/repo/workflows/test.md@v1", + workflowSourceURL: "https://github.com/owner/repo/blob/main/test.md", + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 400, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-400", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.body).toContain("Review body"); + // Footer generated by generate_footer.cjs + expect(callArgs.body).toContain("test-workflow"); + }); + + it("should handle API errors gracefully", async () => { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockRejectedValue(new Error("Validation Failed")); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(false); + expect(result.error).toContain("Validation Failed"); + }); + + it("should submit multiple comments in a single review", async () => { + buffer.addComment({ path: "file1.js", line: 5, body: "Comment 1" }); + buffer.addComment({ path: "file2.js", line: 10, body: "Comment 2" }); + buffer.addComment({ path: "file3.js", line: 15, body: "Comment 3" }); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 42, + pullRequest: { head: { sha: "abc123" } }, + }); + + mockGithub.rest.pulls.createReview.mockResolvedValue({ + data: { + id: 500, + html_url: "https://github.com/owner/repo/pull/42#pullrequestreview-500", + }, + }); + + const result = await buffer.submitReview(); + + expect(result.success).toBe(true); + expect(result.comment_count).toBe(3); + + const callArgs = mockGithub.rest.pulls.createReview.mock.calls[0][0]; + expect(callArgs.comments).toHaveLength(3); + }); + }); + + describe("reset", () => { + it("should clear all state", () => { + buffer.addComment({ path: "test.js", line: 1, body: "comment" }); + buffer.setReviewMetadata("body", "APPROVE"); + buffer.setReviewContext({ + repo: "owner/repo", + repoParts: { owner: "owner", repo: "repo" }, + pullRequestNumber: 1, + pullRequest: { head: { sha: "abc" } }, + }); + + buffer.reset(); + + expect(buffer.hasBufferedComments()).toBe(false); + expect(buffer.getBufferedCount()).toBe(0); + expect(buffer.hasReviewMetadata()).toBe(false); + expect(buffer.getReviewContext()).toBeNull(); + }); + }); +}); diff --git a/actions/setup/js/safe_output_handler_manager.cjs b/actions/setup/js/safe_output_handler_manager.cjs index 720b73f332..4783af00cc 100644 --- a/actions/setup/js/safe_output_handler_manager.cjs +++ b/actions/setup/js/safe_output_handler_manager.cjs @@ -16,6 +16,7 @@ const { generateMissingInfoSections } = require("./missing_info_formatter.cjs"); const { setCollectedMissings } = require("./missing_messages_helper.cjs"); const { writeSafeOutputSummaries } = require("./safe_output_summary.cjs"); const { getIssuesToAssignCopilot } = require("./create_issue.cjs"); +const { createReviewBuffer } = require("./pr_review_buffer.cjs"); /** * Handler map configuration @@ -34,6 +35,7 @@ const HANDLER_MAP = { link_sub_issue: "./link_sub_issue.cjs", update_release: "./update_release.cjs", create_pull_request_review_comment: "./create_pr_review_comment.cjs", + submit_pull_request_review: "./submit_pr_review.cjs", create_pull_request: "./create_pull_request.cjs", push_to_pull_request_branch: "./push_to_pull_request_branch.cjs", update_pull_request: "./update_pull_request.cjs", @@ -86,13 +88,17 @@ function loadConfig() { } } +/** @type {Set} Handler types that participate in the PR review buffer */ +const PR_REVIEW_HANDLER_TYPES = new Set(["create_pull_request_review_comment", "submit_pull_request_review"]); + /** * Load and initialize handlers for enabled safe output types * Calls each handler's factory function (main) to get message processors * @param {Object} config - Safe outputs configuration + * @param {Object} prReviewBuffer - Shared PR review buffer instance * @returns {Promise>} Map of type to message handler function */ -async function loadHandlers(config) { +async function loadHandlers(config, prReviewBuffer) { const messageHandlers = new Map(); core.info("Loading and initializing safe output handlers based on configuration..."); @@ -105,7 +111,13 @@ async function loadHandlers(config) { const handlerModule = require(handlerPath); if (handlerModule && typeof handlerModule.main === "function") { // Call the factory function with config to get the message handler - const handlerConfig = config[type] || {}; + const handlerConfig = { ...(config[type] || {}) }; + + // Inject shared PR review buffer into handlers that need it + if (PR_REVIEW_HANDLER_TYPES.has(type)) { + handlerConfig._prReviewBuffer = prReviewBuffer; + } + const messageHandler = await handlerModule.main(handlerConfig); if (typeof messageHandler !== "function") { @@ -722,8 +734,11 @@ async function main() { core.info(`Found ${agentOutput.items.length} message(s) in agent output`); + // Create the shared PR review buffer instance (no global state) + const prReviewBuffer = createReviewBuffer(); + // Load and initialize handlers based on configuration (factory pattern) - const messageHandlers = await loadHandlers(config); + const messageHandlers = await loadHandlers(config, prReviewBuffer); if (messageHandlers.size === 0) { core.info("No handlers loaded - nothing to process"); @@ -736,6 +751,27 @@ async function main() { // Process all messages in order of appearance const processingResult = await processMessages(messageHandlers, agentOutput.items); + // Finalize buffered PR review — submit when comments or metadata exist + if (prReviewBuffer.hasBufferedComments() || prReviewBuffer.hasReviewMetadata()) { + core.info(`\n=== Finalizing PR Review ===`); + const bufferedCount = prReviewBuffer.getBufferedCount(); + if (bufferedCount > 0) { + core.info(`Submitting ${bufferedCount} buffered review comment(s) as a single PR review`); + } else { + core.info("Submitting PR review (body-only, no inline comments)"); + } + try { + const reviewResult = await prReviewBuffer.submitReview(); + if (reviewResult.success && !reviewResult.skipped) { + core.info(`✓ PR review submitted successfully: ${reviewResult.review_url}`); + } else if (!reviewResult.success) { + core.warning(`✗ Failed to submit PR review: ${reviewResult.error}`); + } + } catch (reviewError) { + core.warning(`✗ Exception while submitting PR review: ${reviewError.message || reviewError}`); + } + } + // Store collected missings in helper module for handlers to access if (processingResult.missings) { setCollectedMissings(processingResult.missings); diff --git a/actions/setup/js/safe_output_type_validator.test.cjs b/actions/setup/js/safe_output_type_validator.test.cjs index e723036aff..32875e9265 100644 --- a/actions/setup/js/safe_output_type_validator.test.cjs +++ b/actions/setup/js/safe_output_type_validator.test.cjs @@ -66,6 +66,13 @@ const SAMPLE_VALIDATION_CONFIG = { side: { type: "string", enum: ["LEFT", "RIGHT"] }, }, }, + submit_pull_request_review: { + defaultMax: 1, + fields: { + body: { type: "string", sanitize: true, maxLength: 65000 }, + event: { type: "string", enum: ["APPROVE", "REQUEST_CHANGES", "COMMENT"] }, + }, + }, link_sub_issue: { defaultMax: 5, customValidation: "parentAndSubDifferent", diff --git a/actions/setup/js/safe_output_unified_handler_manager.cjs b/actions/setup/js/safe_output_unified_handler_manager.cjs index cce0ea8934..c0caecee3f 100644 --- a/actions/setup/js/safe_output_unified_handler_manager.cjs +++ b/actions/setup/js/safe_output_unified_handler_manager.cjs @@ -23,6 +23,7 @@ const { writeSafeOutputSummaries } = require("./safe_output_summary.cjs"); const { getIssuesToAssignCopilot } = require("./create_issue.cjs"); const { sortSafeOutputMessages } = require("./safe_output_topological_sort.cjs"); const { loadCustomSafeOutputJobTypes } = require("./safe_output_helpers.cjs"); +const { createReviewBuffer } = require("./pr_review_buffer.cjs"); /** * Handler map configuration for regular handlers @@ -42,6 +43,7 @@ const HANDLER_MAP = { link_sub_issue: "./link_sub_issue.cjs", update_release: "./update_release.cjs", create_pull_request_review_comment: "./create_pr_review_comment.cjs", + submit_pull_request_review: "./submit_pr_review.cjs", create_pull_request: "./create_pull_request.cjs", push_to_pull_request_branch: "./push_to_pull_request_branch.cjs", update_pull_request: "./update_pull_request.cjs", @@ -175,15 +177,19 @@ async function setupProjectGitHubClient() { return octokit; } +/** @type {Set} Handler types that participate in the PR review buffer */ +const PR_REVIEW_HANDLER_TYPES = new Set(["create_pull_request_review_comment", "submit_pull_request_review"]); + /** * Load and initialize handlers for enabled safe output types * Calls each handler's factory function (main) to get message processors * Regular handlers use the global github object, project handlers use a separate Octokit instance * @param {{regular: Object, project: Object}} configs - Safe outputs configuration for regular and project handlers * @param {Object} projectOctokit - Octokit instance for project handlers (optional, required if project handlers are configured) + * @param {Object} prReviewBuffer - Shared PR review buffer instance * @returns {Promise>} Map of type to message handler function */ -async function loadHandlers(configs, projectOctokit = null) { +async function loadHandlers(configs, projectOctokit = null, prReviewBuffer = null) { const messageHandlers = new Map(); core.info("Loading and initializing safe output handlers based on configuration..."); @@ -195,7 +201,13 @@ async function loadHandlers(configs, projectOctokit = null) { const handlerModule = require(handlerPath); if (handlerModule && typeof handlerModule.main === "function") { // Call the factory function with config to get the message handler - const handlerConfig = configs.regular[type] || {}; + const handlerConfig = { ...(configs.regular[type] || {}) }; + + // Inject shared PR review buffer into handlers that need it + if (PR_REVIEW_HANDLER_TYPES.has(type) && prReviewBuffer) { + handlerConfig._prReviewBuffer = prReviewBuffer; + } + const messageHandler = await handlerModule.main(handlerConfig); if (typeof messageHandler !== "function") { @@ -922,9 +934,12 @@ async function main() { core.info(`Found ${agentOutput.items.length} message(s) in agent output`); + // Create the shared PR review buffer instance (no global state) + const prReviewBuffer = createReviewBuffer(); + // Load and initialize handlers based on configuration (factory pattern) // Regular handlers use the global github object, project handlers use the projectOctokit - const messageHandlers = await loadHandlers(configs, projectOctokit); + const messageHandlers = await loadHandlers(configs, projectOctokit, prReviewBuffer); if (messageHandlers.size === 0) { core.info("No handlers loaded - nothing to process"); @@ -938,6 +953,27 @@ async function main() { // Pass the projectOctokit so project handlers can use it const processingResult = await processMessages(messageHandlers, agentOutput.items, projectOctokit); + // Finalize buffered PR review — submit when comments or metadata exist + if (prReviewBuffer.hasBufferedComments() || prReviewBuffer.hasReviewMetadata()) { + core.info(`\n=== Finalizing PR Review ===`); + const bufferedCount = prReviewBuffer.getBufferedCount(); + if (bufferedCount > 0) { + core.info(`Submitting ${bufferedCount} buffered review comment(s) as a single PR review`); + } else { + core.info("Submitting PR review (body-only, no inline comments)"); + } + try { + const reviewResult = await prReviewBuffer.submitReview(); + if (reviewResult.success && !reviewResult.skipped) { + core.info(`✓ PR review submitted successfully: ${reviewResult.review_url}`); + } else if (!reviewResult.success) { + core.warning(`✗ Failed to submit PR review: ${reviewResult.error}`); + } + } catch (reviewError) { + core.warning(`✗ Exception while submitting PR review: ${reviewError.message || reviewError}`); + } + } + // Store collected missings in helper module for handlers to access if (processingResult.missings) { setCollectedMissings(processingResult.missings); diff --git a/actions/setup/js/safe_outputs_tools.json b/actions/setup/js/safe_outputs_tools.json index 9485637335..137ca1d51e 100644 --- a/actions/setup/js/safe_outputs_tools.json +++ b/actions/setup/js/safe_outputs_tools.json @@ -236,6 +236,25 @@ "additionalProperties": false } }, + { + "name": "submit_pull_request_review", + "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Use APPROVE to approve the PR, REQUEST_CHANGES to request changes, or COMMENT for general feedback without a decision. If you don't call this tool, review comments are still submitted as a COMMENT review.", + "inputSchema": { + "type": "object", + "properties": { + "body": { + "type": "string", + "description": "Overall review summary in Markdown. Provide a high-level assessment of the changes. Required for APPROVE and REQUEST_CHANGES events; optional for COMMENT." + }, + "event": { + "type": "string", + "enum": ["APPROVE", "REQUEST_CHANGES", "COMMENT"], + "description": "Review decision: APPROVE to approve the pull request, REQUEST_CHANGES to formally request changes before merging, or COMMENT for general feedback without a formal decision. Defaults to COMMENT when omitted." + } + }, + "additionalProperties": false + } + }, { "name": "create_code_scanning_alert", "description": "Create a code scanning alert for security vulnerabilities, code quality issues, or other findings. Alerts appear in the repository's Security tab and integrate with GitHub's security features. Use this for automated security analysis results.", diff --git a/actions/setup/js/safe_outputs_type_validation.test.cjs b/actions/setup/js/safe_outputs_type_validation.test.cjs index f80cead279..553d094940 100644 --- a/actions/setup/js/safe_outputs_type_validation.test.cjs +++ b/actions/setup/js/safe_outputs_type_validation.test.cjs @@ -13,6 +13,7 @@ describe("Safe Output Type Validation", () => { "create_discussion.cjs": "create_discussion", "push_to_pull_request_branch.cjs": "push_to_pull_request_branch", "create_pull_request.cjs": "create_pull_request", + "submit_pr_review.cjs": "submit_pull_request_review", }).forEach(([fileName, expectedType]) => { it(`should use underscores in type filter for ${fileName}`, () => { const filePath = path.join(process.cwd(), fileName), @@ -27,15 +28,25 @@ describe("Safe Output Type Validation", () => { it("should validate schema uses underscores", () => { const schemaPath = path.join(process.cwd(), "..", "..", "..", "schemas", "agent-output.json"), schemaContent = fs.readFileSync(schemaPath, "utf8"); - ["create_issue", "add_comment", "create_pull_request", "add_labels", "update_issue", "push_to_pull_request_branch", "create_pull_request_review_comment", "create_discussion", "missing_tool", "create_code_scanning_alert"].forEach( - type => { - const hasType = schemaContent.includes(`"const": "${type}"`); - expect(hasType).toBe(!0); - const dashType = type.replace(/_/g, "-"), - hasDashType = schemaContent.includes(`"const": "${dashType}"`); - expect(hasDashType).toBe(!1); - } - ); + [ + "create_issue", + "add_comment", + "create_pull_request", + "add_labels", + "update_issue", + "push_to_pull_request_branch", + "create_pull_request_review_comment", + "submit_pull_request_review", + "create_discussion", + "missing_tool", + "create_code_scanning_alert", + ].forEach(type => { + const hasType = schemaContent.includes(`"const": "${type}"`); + expect(hasType).toBe(!0); + const dashType = type.replace(/_/g, "-"), + hasDashType = schemaContent.includes(`"const": "${dashType}"`); + expect(hasDashType).toBe(!1); + }); }), it("should validate MCP server normalizes types to underscores", () => { const appendPath = path.join(process.cwd(), "safe_outputs_append.cjs"), @@ -44,10 +55,20 @@ describe("Safe Output Type Validation", () => { const toolsJsonPath = path.join(process.cwd(), "safe_outputs_tools.json"), toolsContent = fs.readFileSync(toolsJsonPath, "utf8"), actualToolNames = JSON.parse(toolsContent).map(t => t.name); - ["create_issue", "create_discussion", "add_comment", "create_pull_request", "create_pull_request_review_comment", "create_code_scanning_alert", "add_labels", "update_issue", "push_to_pull_request_branch", "upload_asset"].forEach( - toolName => { - expect(actualToolNames).toContain(toolName); - } - ); + [ + "create_issue", + "create_discussion", + "add_comment", + "create_pull_request", + "create_pull_request_review_comment", + "submit_pull_request_review", + "create_code_scanning_alert", + "add_labels", + "update_issue", + "push_to_pull_request_branch", + "upload_asset", + ].forEach(toolName => { + expect(actualToolNames).toContain(toolName); + }); })); }); diff --git a/actions/setup/js/submit_pr_review.cjs b/actions/setup/js/submit_pr_review.cjs new file mode 100644 index 0000000000..fd1b91202d --- /dev/null +++ b/actions/setup/js/submit_pr_review.cjs @@ -0,0 +1,106 @@ +// @ts-check +/// + +/** + * @typedef {import('./types/handler-factory').HandlerFactoryFunction} HandlerFactoryFunction + */ + +/** @type {string} Safe output type handled by this module */ +const HANDLER_TYPE = "submit_pull_request_review"; + +/** @type {Set} Valid review event types */ +const VALID_EVENTS = new Set(["APPROVE", "REQUEST_CHANGES", "COMMENT"]); + +/** + * Main handler factory for submit_pull_request_review + * Returns a message handler that stores review metadata (body and event) + * in the shared PR review buffer. The actual review submission happens + * during the handler manager's finalization step. + * + * The PR review buffer instance is passed via config._prReviewBuffer. + * + * @type {HandlerFactoryFunction} + */ +async function main(config = {}) { + const maxCount = config.max || 1; + const buffer = config._prReviewBuffer; + + if (!buffer) { + core.warning("submit_pull_request_review: No PR review buffer provided in config"); + return async function handleSubmitPRReview() { + return { success: false, error: "No PR review buffer available" }; + }; + } + + core.info(`Submit PR review handler initialized: max=${maxCount}`); + + let processedCount = 0; + + /** + * Message handler that stores review metadata + * @param {Object} message - The submit_pull_request_review message + * @param {Object} resolvedTemporaryIds - Map of temporary IDs + * @returns {Promise} Result with success status + */ + return async function handleSubmitPRReview(message, resolvedTemporaryIds) { + if (processedCount >= maxCount) { + core.warning(`Skipping submit_pull_request_review: max count of ${maxCount} reached`); + return { + success: false, + error: `Max count of ${maxCount} reached`, + }; + } + + // Validate event field — default to COMMENT when not provided + const event = message.event ? message.event.toUpperCase() : "COMMENT"; + if (!VALID_EVENTS.has(event)) { + core.warning(`Invalid review event: ${message.event}. Must be one of: APPROVE, REQUEST_CHANGES, COMMENT`); + return { + success: false, + error: `Invalid review event: ${message.event}. Must be one of: APPROVE, REQUEST_CHANGES, COMMENT`, + }; + } + + // Body is required for APPROVE and REQUEST_CHANGES + const body = message.body || ""; + if ((event === "APPROVE" || event === "REQUEST_CHANGES") && !body) { + core.warning(`Review body is required for event type ${event}`); + return { + success: false, + error: `Review body is required for event type ${event}`, + }; + } + + // Only increment after validation passes + processedCount++; + + core.info(`Setting review metadata: event=${event}, bodyLength=${body.length}`); + + // Store the review metadata in the shared buffer + buffer.setReviewMetadata(body, event); + + // Ensure review context is set for body-only reviews (no inline comments). + // If create_pull_request_review_comment already set context, this is a no-op. + if (!buffer.getReviewContext() && typeof context !== "undefined" && context && context.payload) { + const pr = context.payload.pull_request; + if (pr && pr.head && pr.head.sha) { + const repo = `${context.repo.owner}/${context.repo.repo}`; + buffer.setReviewContext({ + repo, + repoParts: { owner: context.repo.owner, repo: context.repo.repo }, + pullRequestNumber: pr.number, + pullRequest: pr, + }); + core.info(`Set review context from triggering PR: ${repo}#${pr.number}`); + } + } + + return { + success: true, + event: event, + body_length: body.length, + }; + }; +} + +module.exports = { main }; diff --git a/actions/setup/js/submit_pr_review.test.cjs b/actions/setup/js/submit_pr_review.test.cjs new file mode 100644 index 0000000000..b551560f88 --- /dev/null +++ b/actions/setup/js/submit_pr_review.test.cjs @@ -0,0 +1,314 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; + +const mockCore = { + debug: vi.fn(), + info: vi.fn(), + warning: vi.fn(), + error: vi.fn(), + setFailed: vi.fn(), + setOutput: vi.fn(), + summary: { + addRaw: vi.fn().mockReturnThis(), + write: vi.fn().mockResolvedValue(), + }, +}; + +global.core = mockCore; + +const { createReviewBuffer } = require("./pr_review_buffer.cjs"); + +describe("submit_pr_review (Handler Factory Architecture)", () => { + let handler; + let buffer; + + beforeEach(async () => { + vi.clearAllMocks(); + + // Create a fresh buffer for each test (factory pattern, no global state) + buffer = createReviewBuffer(); + + const { main } = require("./submit_pr_review.cjs"); + handler = await main({ max: 1, _prReviewBuffer: buffer }); + }); + + it("should return a function from main()", async () => { + const { main } = require("./submit_pr_review.cjs"); + const localBuffer = createReviewBuffer(); + const result = await main({ _prReviewBuffer: localBuffer }); + expect(typeof result).toBe("function"); + }); + + it("should return error when no buffer provided", async () => { + const { main } = require("./submit_pr_review.cjs"); + const noBufferHandler = await main({}); + const result = await noBufferHandler({}, {}); + expect(result.success).toBe(false); + expect(result.error).toContain("No PR review buffer available"); + }); + + it("should set review metadata for APPROVE event", async () => { + const message = { + type: "submit_pull_request_review", + body: "LGTM! Great changes.", + event: "APPROVE", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("APPROVE"); + expect(result.body_length).toBe(20); + expect(buffer.hasReviewMetadata()).toBe(true); + }); + + it("should set review metadata for REQUEST_CHANGES event", async () => { + const message = { + type: "submit_pull_request_review", + body: "Please fix the issues.", + event: "REQUEST_CHANGES", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("REQUEST_CHANGES"); + }); + + it("should set review metadata for COMMENT event", async () => { + const message = { + type: "submit_pull_request_review", + body: "Some general feedback.", + event: "COMMENT", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("COMMENT"); + }); + + it("should normalize event to uppercase", async () => { + const message = { + type: "submit_pull_request_review", + body: "Looks good", + event: "approve", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("APPROVE"); + }); + + it("should default event to COMMENT when missing", async () => { + const message = { + type: "submit_pull_request_review", + body: "Some feedback", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("COMMENT"); + }); + + it("should reject invalid event values", async () => { + const message = { + type: "submit_pull_request_review", + body: "Bad event", + event: "INVALID", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Invalid review event"); + }); + + it("should require body for APPROVE event", async () => { + const message = { + type: "submit_pull_request_review", + body: "", + event: "APPROVE", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Review body is required"); + }); + + it("should require body for REQUEST_CHANGES event", async () => { + const message = { + type: "submit_pull_request_review", + body: "", + event: "REQUEST_CHANGES", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(false); + expect(result.error).toContain("Review body is required"); + }); + + it("should allow empty body for COMMENT event", async () => { + const message = { + type: "submit_pull_request_review", + event: "COMMENT", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("COMMENT"); + }); + + it("should allow no event and no body (defaults to COMMENT)", async () => { + const message = { + type: "submit_pull_request_review", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + expect(result.event).toBe("COMMENT"); + expect(result.body_length).toBe(0); + }); + + it("should respect max count configuration", async () => { + const message1 = { + type: "submit_pull_request_review", + body: "First review", + event: "COMMENT", + }; + + const message2 = { + type: "submit_pull_request_review", + body: "Second review", + event: "COMMENT", + }; + + // First call should succeed + const result1 = await handler(message1, {}); + expect(result1.success).toBe(true); + + // Second call should fail (max=1) + const result2 = await handler(message2, {}); + expect(result2.success).toBe(false); + expect(result2.error).toContain("Max count"); + }); + + it("should not consume max count slot on validation failure", async () => { + // Invalid event should not consume a slot + const invalidMessage = { + type: "submit_pull_request_review", + body: "Bad event", + event: "INVALID", + }; + + const result1 = await handler(invalidMessage, {}); + expect(result1.success).toBe(false); + + // Valid message should still succeed since the invalid one didn't count + const validMessage = { + type: "submit_pull_request_review", + body: "Good review", + event: "APPROVE", + }; + + const result2 = await handler(validMessage, {}); + expect(result2.success).toBe(true); + expect(result2.event).toBe("APPROVE"); + }); + + it("should not consume max count slot when body is missing for APPROVE", async () => { + // Missing body for APPROVE should not consume a slot + const noBodyMessage = { + type: "submit_pull_request_review", + body: "", + event: "APPROVE", + }; + + const result1 = await handler(noBodyMessage, {}); + expect(result1.success).toBe(false); + + // Valid message should still succeed + const validMessage = { + type: "submit_pull_request_review", + body: "Now with body", + event: "APPROVE", + }; + + const result2 = await handler(validMessage, {}); + expect(result2.success).toBe(true); + }); + + it("should set review context from triggering PR for body-only reviews", async () => { + // Simulate a PR trigger context + global.context = { + repo: { owner: "test-owner", repo: "test-repo" }, + payload: { + pull_request: { number: 42, head: { sha: "abc123" } }, + }, + }; + + const localBuffer = createReviewBuffer(); + const { main } = require("./submit_pr_review.cjs"); + const localHandler = await main({ max: 1, _prReviewBuffer: localBuffer }); + + const message = { + type: "submit_pull_request_review", + body: "LGTM!", + event: "APPROVE", + }; + + const result = await localHandler(message, {}); + + expect(result.success).toBe(true); + expect(localBuffer.hasReviewMetadata()).toBe(true); + + // The handler should have set review context from triggering PR + const ctx = localBuffer.getReviewContext(); + expect(ctx).not.toBeNull(); + expect(ctx.repo).toBe("test-owner/test-repo"); + expect(ctx.pullRequestNumber).toBe(42); + + // Clean up + delete global.context; + }); + + it("should not override existing review context from comments", async () => { + // Pre-set context as if a comment handler already set it + buffer.setReviewContext({ + repo: "comment-owner/comment-repo", + repoParts: { owner: "comment-owner", repo: "comment-repo" }, + pullRequestNumber: 99, + pullRequest: { head: { sha: "comment-sha" } }, + }); + + global.context = { + repo: { owner: "trigger-owner", repo: "trigger-repo" }, + payload: { + pull_request: { number: 1, head: { sha: "trigger-sha" } }, + }, + }; + + const message = { + type: "submit_pull_request_review", + body: "Review body", + event: "COMMENT", + }; + + const result = await handler(message, {}); + + expect(result.success).toBe(true); + + // Context should still be from the comment handler, not overridden + const ctx = buffer.getReviewContext(); + expect(ctx.repo).toBe("comment-owner/comment-repo"); + expect(ctx.pullRequestNumber).toBe(99); + + // Clean up + delete global.context; + }); +}); diff --git a/docs/src/content/docs/reference/frontmatter-full.md b/docs/src/content/docs/reference/frontmatter-full.md index b1f2fcdfa1..e0eae18013 100644 --- a/docs/src/content/docs/reference/frontmatter-full.md +++ b/docs/src/content/docs/reference/frontmatter-full.md @@ -2686,6 +2686,29 @@ safe-outputs: # Option 2: Enable PR review comment creation with default configuration create-pull-request-review-comment: null + # Enable AI agents to submit consolidated pull request reviews with a status + # decision. Works with create-pull-request-review-comment to batch inline comments + # into a single review. + # (optional) + # This field supports multiple formats (oneOf): + + # Option 1: Configuration for submitting a consolidated PR review with a status + # decision (APPROVE, REQUEST_CHANGES, COMMENT). All + # create-pull-request-review-comment outputs are collected and submitted as part + # of this review. + submit-pull-request-review: + # Maximum number of reviews to submit (default: 1) + # (optional) + max: 1 + + # GitHub token to use for this specific output type. Overrides global github-token + # if specified. + # (optional) + github-token: "${{ secrets.GITHUB_TOKEN }}" + + # Option 2: Enable PR review submission with default configuration + submit-pull-request-review: null + # Enable AI agents to create GitHub Advanced Security code scanning alerts for # detected vulnerabilities or security issues. # (optional) diff --git a/docs/src/content/docs/reference/safe-outputs.md b/docs/src/content/docs/reference/safe-outputs.md index beb763ed66..88398396e5 100644 --- a/docs/src/content/docs/reference/safe-outputs.md +++ b/docs/src/content/docs/reference/safe-outputs.md @@ -675,7 +675,7 @@ safe-outputs: ### PR Review Comments (`create-pull-request-review-comment:`) -Creates review comments on specific code lines in PRs. Supports single-line and multi-line comments. +Creates review comments on specific code lines in PRs. Supports single-line and multi-line comments. Comments are buffered and submitted as a single PR review (see `submit-pull-request-review` below). ```yaml wrap safe-outputs: @@ -686,6 +686,22 @@ safe-outputs: target-repo: "owner/repo" # cross-repository ``` +### Submit PR Review (`submit-pull-request-review:`) + +Submits a consolidated pull request review with a status decision. All `create-pull-request-review-comment` outputs are automatically collected and included as inline comments in the review. + +If the agent calls `submit_pull_request_review`, it can specify a review `body` and `event` (APPROVE, REQUEST_CHANGES, or COMMENT). Both fields are optional — `event` defaults to COMMENT when omitted, and `body` is only required for APPROVE and REQUEST_CHANGES decisions. The agent can also submit a body-only review (e.g., APPROVE) without any inline comments. + +If the agent does not call `submit_pull_request_review` at all, buffered comments are still submitted as a COMMENT review automatically. + +```yaml wrap +safe-outputs: + create-pull-request-review-comment: + max: 10 + submit-pull-request-review: + max: 1 # max reviews to submit (default: 1) +``` + ### Code Scanning Alerts (`create-code-scanning-alert:`) Creates security advisories in SARIF format and submits to GitHub Code Scanning. Supports severity: error, warning, info, note. diff --git a/pkg/parser/schemas/main_workflow_schema.json b/pkg/parser/schemas/main_workflow_schema.json index abf60d6a3b..b9eb951f38 100644 --- a/pkg/parser/schemas/main_workflow_schema.json +++ b/pkg/parser/schemas/main_workflow_schema.json @@ -3847,7 +3847,7 @@ }, "safe-outputs": { "type": "object", - "$comment": "Required if workflow creates or modifies GitHub resources. Operations requiring safe-outputs: autofix-code-scanning-alert, add-comment, add-labels, add-reviewer, assign-milestone, assign-to-agent, close-discussion, close-issue, close-pull-request, create-agent-session, create-agent-task (deprecated, use create-agent-session), create-code-scanning-alert, create-discussion, create-issue, create-project-status-update, create-pull-request, create-pull-request-review-comment, dispatch-workflow, hide-comment, link-sub-issue, mark-pull-request-as-ready-for-review, missing-tool, noop, push-to-pull-request-branch, remove-labels, threat-detection, update-discussion, update-issue, update-project, update-pull-request, update-release, upload-asset. See documentation for complete details.", + "$comment": "Required if workflow creates or modifies GitHub resources. Operations requiring safe-outputs: autofix-code-scanning-alert, add-comment, add-labels, add-reviewer, assign-milestone, assign-to-agent, close-discussion, close-issue, close-pull-request, create-agent-session, create-agent-task (deprecated, use create-agent-session), create-code-scanning-alert, create-discussion, create-issue, create-project-status-update, create-pull-request, create-pull-request-review-comment, dispatch-workflow, hide-comment, link-sub-issue, mark-pull-request-as-ready-for-review, missing-tool, noop, push-to-pull-request-branch, remove-labels, submit-pull-request-review, threat-detection, update-discussion, update-issue, update-project, update-pull-request, update-release, upload-asset. See documentation for complete details.", "description": "Safe output processing configuration that automatically creates GitHub issues, comments, and pull requests from AI workflow output without requiring write permissions in the main job", "examples": [ { @@ -4969,6 +4969,32 @@ ], "description": "Enable AI agents to add review comments to specific lines in pull request diffs during code review workflows." }, + "submit-pull-request-review": { + "oneOf": [ + { + "type": "object", + "description": "Configuration for submitting a consolidated PR review with a status decision (APPROVE, REQUEST_CHANGES, COMMENT). All create-pull-request-review-comment outputs are collected and submitted as part of this review.", + "properties": { + "max": { + "type": "integer", + "description": "Maximum number of reviews to submit (default: 1)", + "minimum": 1, + "maximum": 10 + }, + "github-token": { + "$ref": "#/$defs/github_token", + "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." + } + }, + "additionalProperties": false + }, + { + "type": "null", + "description": "Enable PR review submission with default configuration" + } + ], + "description": "Enable AI agents to submit consolidated pull request reviews with a status decision. Works with create-pull-request-review-comment to batch inline comments into a single review." + }, "create-code-scanning-alert": { "oneOf": [ { diff --git a/pkg/workflow/compiler_safe_outputs_config.go b/pkg/workflow/compiler_safe_outputs_config.go index bb73f63482..cd7003933b 100644 --- a/pkg/workflow/compiler_safe_outputs_config.go +++ b/pkg/workflow/compiler_safe_outputs_config.go @@ -305,6 +305,15 @@ var handlerRegistry = map[string]handlerBuilder{ AddStringSlice("allowed_repos", c.AllowedRepos). Build() }, + "submit_pull_request_review": func(cfg *SafeOutputsConfig) map[string]any { + if cfg.SubmitPullRequestReview == nil { + return nil + } + c := cfg.SubmitPullRequestReview + return newHandlerConfigBuilder(). + AddIfPositive("max", c.Max). + Build() + }, "create_pull_request": func(cfg *SafeOutputsConfig) map[string]any { if cfg.CreatePullRequests == nil { return nil diff --git a/pkg/workflow/compiler_safe_outputs_job.go b/pkg/workflow/compiler_safe_outputs_job.go index c347d5a8fb..b4b21a576b 100644 --- a/pkg/workflow/compiler_safe_outputs_job.go +++ b/pkg/workflow/compiler_safe_outputs_job.go @@ -131,6 +131,7 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa data.SafeOutputs.LinkSubIssue != nil || data.SafeOutputs.UpdateRelease != nil || data.SafeOutputs.CreatePullRequestReviewComments != nil || + data.SafeOutputs.SubmitPullRequestReview != nil || data.SafeOutputs.CreatePullRequests != nil || data.SafeOutputs.PushToPullRequestBranch != nil || data.SafeOutputs.UpdatePullRequests != nil || @@ -195,7 +196,7 @@ func (c *Compiler) buildConsolidatedSafeOutputsJob(data *WorkflowData, mainJobNa if data.SafeOutputs.UpdateRelease != nil { permissions.Merge(NewPermissionsContentsWrite()) } - if data.SafeOutputs.CreatePullRequestReviewComments != nil { + if data.SafeOutputs.CreatePullRequestReviewComments != nil || data.SafeOutputs.SubmitPullRequestReview != nil { permissions.Merge(NewPermissionsContentsReadPRWrite()) } if data.SafeOutputs.CreatePullRequests != nil { diff --git a/pkg/workflow/compiler_types.go b/pkg/workflow/compiler_types.go index 3f97daca83..0c9775f523 100644 --- a/pkg/workflow/compiler_types.go +++ b/pkg/workflow/compiler_types.go @@ -467,6 +467,7 @@ type SafeOutputsConfig struct { AddComments *AddCommentsConfig `yaml:"add-comments,omitempty"` CreatePullRequests *CreatePullRequestsConfig `yaml:"create-pull-requests,omitempty"` CreatePullRequestReviewComments *CreatePullRequestReviewCommentsConfig `yaml:"create-pull-request-review-comments,omitempty"` + SubmitPullRequestReview *SubmitPullRequestReviewConfig `yaml:"submit-pull-request-review,omitempty"` // Submit a PR review with status (APPROVE, REQUEST_CHANGES, COMMENT) CreateCodeScanningAlerts *CreateCodeScanningAlertsConfig `yaml:"create-code-scanning-alerts,omitempty"` AutofixCodeScanningAlert *AutofixCodeScanningAlertConfig `yaml:"autofix-code-scanning-alert,omitempty"` AddLabels *AddLabelsConfig `yaml:"add-labels,omitempty"` diff --git a/pkg/workflow/imports.go b/pkg/workflow/imports.go index 274dd2ae4b..50d5a58926 100644 --- a/pkg/workflow/imports.go +++ b/pkg/workflow/imports.go @@ -443,6 +443,8 @@ func hasSafeOutputType(config *SafeOutputsConfig, key string) bool { return config.CreatePullRequests != nil case "create-pull-request-review-comment": return config.CreatePullRequestReviewComments != nil + case "submit-pull-request-review": + return config.SubmitPullRequestReview != nil case "create-code-scanning-alert": return config.CreateCodeScanningAlerts != nil case "add-labels": @@ -526,6 +528,9 @@ func mergeSafeOutputConfig(result *SafeOutputsConfig, config map[string]any, c * if result.CreatePullRequestReviewComments == nil && importedConfig.CreatePullRequestReviewComments != nil { result.CreatePullRequestReviewComments = importedConfig.CreatePullRequestReviewComments } + if result.SubmitPullRequestReview == nil && importedConfig.SubmitPullRequestReview != nil { + result.SubmitPullRequestReview = importedConfig.SubmitPullRequestReview + } if result.CreateCodeScanningAlerts == nil && importedConfig.CreateCodeScanningAlerts != nil { result.CreateCodeScanningAlerts = importedConfig.CreateCodeScanningAlerts } diff --git a/pkg/workflow/js/safe_outputs_tools.json b/pkg/workflow/js/safe_outputs_tools.json index 8322f20d89..fe9cb1b2e5 100644 --- a/pkg/workflow/js/safe_outputs_tools.json +++ b/pkg/workflow/js/safe_outputs_tools.json @@ -288,6 +288,29 @@ "additionalProperties": false } }, + { + "name": "submit_pull_request_review", + "description": "Submit a pull request review with a status decision. All create_pull_request_review_comment outputs are automatically collected and included as inline comments in this review. Use APPROVE to approve the PR, REQUEST_CHANGES to request changes, or COMMENT for general feedback without a decision. If you don't call this tool, review comments are still submitted as a COMMENT review.", + "inputSchema": { + "type": "object", + "properties": { + "body": { + "type": "string", + "description": "Overall review summary in Markdown. Provide a high-level assessment of the changes. Required for APPROVE and REQUEST_CHANGES events; optional for COMMENT." + }, + "event": { + "type": "string", + "enum": [ + "APPROVE", + "REQUEST_CHANGES", + "COMMENT" + ], + "description": "Review decision: APPROVE to approve the pull request, REQUEST_CHANGES to formally request changes before merging, or COMMENT for general feedback without a formal decision. Defaults to COMMENT when omitted." + } + }, + "additionalProperties": false + } + }, { "name": "create_code_scanning_alert", "description": "Create a code scanning alert for security vulnerabilities, code quality issues, or other findings. Alerts appear in the repository's Security tab and integrate with GitHub's security features. Use this for automated security analysis results.", diff --git a/pkg/workflow/safe_output_validation_config.go b/pkg/workflow/safe_output_validation_config.go index 522ca613db..876674560d 100644 --- a/pkg/workflow/safe_output_validation_config.go +++ b/pkg/workflow/safe_output_validation_config.go @@ -153,6 +153,13 @@ var ValidationConfig = map[string]TypeValidationConfig{ "side": {Type: "string", Enum: []string{"LEFT", "RIGHT"}}, }, }, + "submit_pull_request_review": { + DefaultMax: 1, + Fields: map[string]FieldValidation{ + "body": {Type: "string", Sanitize: true, MaxLength: MaxBodyLength}, + "event": {Type: "string", Enum: []string{"APPROVE", "REQUEST_CHANGES", "COMMENT"}}, + }, + }, "create_discussion": { DefaultMax: 1, Fields: map[string]FieldValidation{ diff --git a/pkg/workflow/safe_output_validation_config_test.go b/pkg/workflow/safe_output_validation_config_test.go index d1db637304..c89252d4eb 100644 --- a/pkg/workflow/safe_output_validation_config_test.go +++ b/pkg/workflow/safe_output_validation_config_test.go @@ -36,6 +36,7 @@ func TestGetValidationConfigJSON(t *testing.T) { "update_pull_request", "push_to_pull_request_branch", "create_pull_request_review_comment", + "submit_pull_request_review", "create_discussion", "close_discussion", "close_issue", diff --git a/pkg/workflow/safe_outputs_config.go b/pkg/workflow/safe_outputs_config.go index e0bc9ac20f..d6c4b05f4e 100644 --- a/pkg/workflow/safe_outputs_config.go +++ b/pkg/workflow/safe_outputs_config.go @@ -98,6 +98,12 @@ func (c *Compiler) extractSafeOutputsConfig(frontmatter map[string]any) *SafeOut config.CreatePullRequestReviewComments = prReviewCommentsConfig } + // Handle submit-pull-request-review + submitPRReviewConfig := c.parseSubmitPullRequestReviewConfig(outputMap) + if submitPRReviewConfig != nil { + config.SubmitPullRequestReview = submitPRReviewConfig + } + // Handle create-code-scanning-alert securityReportsConfig := c.parseCodeScanningAlertsConfig(outputMap) if securityReportsConfig != nil { diff --git a/pkg/workflow/safe_outputs_config_generation.go b/pkg/workflow/safe_outputs_config_generation.go index d4e3e5030c..b5c29368bf 100644 --- a/pkg/workflow/safe_outputs_config_generation.go +++ b/pkg/workflow/safe_outputs_config_generation.go @@ -155,6 +155,12 @@ func generateSafeOutputsConfig(data *WorkflowData) string { 10, // default max ) } + if data.SafeOutputs.SubmitPullRequestReview != nil { + safeOutputsConfig["submit_pull_request_review"] = generateMaxConfig( + data.SafeOutputs.SubmitPullRequestReview.Max, + 1, // default max + ) + } if data.SafeOutputs.CreateCodeScanningAlerts != nil { safeOutputsConfig["create_code_scanning_alert"] = generateMaxConfig( data.SafeOutputs.CreateCodeScanningAlerts.Max, @@ -623,6 +629,9 @@ func generateFilteredToolsJSON(data *WorkflowData, markdownPath string) (string, if data.SafeOutputs.CreatePullRequestReviewComments != nil { enabledTools["create_pull_request_review_comment"] = true } + if data.SafeOutputs.SubmitPullRequestReview != nil { + enabledTools["submit_pull_request_review"] = true + } if data.SafeOutputs.CreateCodeScanningAlerts != nil { enabledTools["create_code_scanning_alert"] = true } diff --git a/pkg/workflow/safe_outputs_config_helpers_reflection.go b/pkg/workflow/safe_outputs_config_helpers_reflection.go index 017ae8fecb..3ee430001a 100644 --- a/pkg/workflow/safe_outputs_config_helpers_reflection.go +++ b/pkg/workflow/safe_outputs_config_helpers_reflection.go @@ -21,6 +21,7 @@ var safeOutputFieldMapping = map[string]string{ "AddComments": "add_comment", "CreatePullRequests": "create_pull_request", "CreatePullRequestReviewComments": "create_pull_request_review_comment", + "SubmitPullRequestReview": "submit_pull_request_review", "CreateCodeScanningAlerts": "create_code_scanning_alert", "AddLabels": "add_labels", "RemoveLabels": "remove_labels", diff --git a/pkg/workflow/safe_outputs_test.go b/pkg/workflow/safe_outputs_test.go index c4e24d51a9..8ec389f0c0 100644 --- a/pkg/workflow/safe_outputs_test.go +++ b/pkg/workflow/safe_outputs_test.go @@ -88,6 +88,13 @@ func TestHasSafeOutputsEnabled(t *testing.T) { }, expected: true, }, + { + name: "submit pull request review enabled", + safeOutputs: &SafeOutputsConfig{ + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{}, + }, + expected: true, + }, { name: "create code scanning alerts enabled", safeOutputs: &SafeOutputsConfig{ @@ -1048,20 +1055,21 @@ func TestGetEnabledSafeOutputToolNames(t *testing.T) { { name: "all standard tools are sorted", safeOutputs: &SafeOutputsConfig{ - CreateIssues: &CreateIssuesConfig{}, - CreateAgentSessions: &CreateAgentSessionConfig{}, - CreateDiscussions: &CreateDiscussionsConfig{}, - CloseDiscussions: &CloseDiscussionsConfig{}, - CloseIssues: &CloseIssuesConfig{}, - ClosePullRequests: &ClosePullRequestsConfig{}, - AddComments: &AddCommentsConfig{}, - CreatePullRequests: &CreatePullRequestsConfig{}, - AddLabels: &AddLabelsConfig{}, - AddReviewer: &AddReviewerConfig{}, - AssignMilestone: &AssignMilestoneConfig{}, - UpdateIssues: &UpdateIssuesConfig{}, - UpdatePullRequests: &UpdatePullRequestsConfig{}, - NoOp: &NoOpConfig{}, + CreateIssues: &CreateIssuesConfig{}, + CreateAgentSessions: &CreateAgentSessionConfig{}, + CreateDiscussions: &CreateDiscussionsConfig{}, + CloseDiscussions: &CloseDiscussionsConfig{}, + CloseIssues: &CloseIssuesConfig{}, + ClosePullRequests: &ClosePullRequestsConfig{}, + AddComments: &AddCommentsConfig{}, + CreatePullRequests: &CreatePullRequestsConfig{}, + AddLabels: &AddLabelsConfig{}, + AddReviewer: &AddReviewerConfig{}, + AssignMilestone: &AssignMilestoneConfig{}, + UpdateIssues: &UpdateIssuesConfig{}, + UpdatePullRequests: &UpdatePullRequestsConfig{}, + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{}, + NoOp: &NoOpConfig{}, }, // Expected order is alphabetical expected: []string{ @@ -1077,6 +1085,7 @@ func TestGetEnabledSafeOutputToolNames(t *testing.T) { "create_issue", "create_pull_request", "noop", + "submit_pull_request_review", "update_issue", "update_pull_request", }, diff --git a/pkg/workflow/safe_outputs_tools_test.go b/pkg/workflow/safe_outputs_tools_test.go index 4ce965491b..e1dc5155cf 100644 --- a/pkg/workflow/safe_outputs_tools_test.go +++ b/pkg/workflow/safe_outputs_tools_test.go @@ -78,6 +78,15 @@ func TestGenerateFilteredToolsJSON(t *testing.T) { }, expectedTools: []string{"create_pull_request_review_comment"}, }, + { + name: "submit pull request review enabled", + safeOutputs: &SafeOutputsConfig{ + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{ + BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 1}, + }, + }, + expectedTools: []string{"submit_pull_request_review"}, + }, { name: "create code scanning alerts enabled", safeOutputs: &SafeOutputsConfig{ @@ -152,6 +161,7 @@ func TestGenerateFilteredToolsJSON(t *testing.T) { AddComments: &AddCommentsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 10}}, CreatePullRequests: &CreatePullRequestsConfig{}, CreatePullRequestReviewComments: &CreatePullRequestReviewCommentsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 5}}, + SubmitPullRequestReview: &SubmitPullRequestReviewConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 1}}, CreateCodeScanningAlerts: &CreateCodeScanningAlertsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 100}}, AddLabels: &AddLabelsConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3}}, AddReviewer: &AddReviewerConfig{BaseSafeOutputConfig: BaseSafeOutputConfig{Max: 3}}, @@ -167,6 +177,7 @@ func TestGenerateFilteredToolsJSON(t *testing.T) { "add_comment", "create_pull_request", "create_pull_request_review_comment", + "submit_pull_request_review", "create_code_scanning_alert", "add_labels", "add_reviewer", @@ -283,6 +294,7 @@ func TestGetSafeOutputsToolsJSON(t *testing.T) { "add_comment", "create_pull_request", "create_pull_request_review_comment", + "submit_pull_request_review", "create_code_scanning_alert", "add_labels", "remove_labels", diff --git a/pkg/workflow/submit_pr_review.go b/pkg/workflow/submit_pr_review.go new file mode 100644 index 0000000000..334b6cdeb4 --- /dev/null +++ b/pkg/workflow/submit_pr_review.go @@ -0,0 +1,38 @@ +package workflow + +import ( + "github.com/github/gh-aw/pkg/logger" +) + +var submitPRReviewLog = logger.New("workflow:submit_pr_review") + +// SubmitPullRequestReviewConfig holds configuration for submitting a GitHub pull request review +// This works in conjunction with create-pull-request-review-comment: all review comments +// are collected and submitted as a single PR review with the configured event type. +// If this safe output type is not configured, review comments default to event: "COMMENT". +type SubmitPullRequestReviewConfig struct { + BaseSafeOutputConfig `yaml:",inline"` +} + +// parseSubmitPullRequestReviewConfig handles submit-pull-request-review configuration +func (c *Compiler) parseSubmitPullRequestReviewConfig(outputMap map[string]any) *SubmitPullRequestReviewConfig { + if _, exists := outputMap["submit-pull-request-review"]; !exists { + submitPRReviewLog.Printf("Configuration not found") + return nil + } + + submitPRReviewLog.Printf("Parsing submit PR review configuration") + + configData := outputMap["submit-pull-request-review"] + config := &SubmitPullRequestReviewConfig{} + + if configMap, ok := configData.(map[string]any); ok { + // Parse common base fields with default max of 1 + c.parseBaseSafeOutputConfig(configMap, &config.BaseSafeOutputConfig, 1) + } else { + // If configData is nil or not a map, set the default max + config.Max = 1 + } + + return config +} diff --git a/pkg/workflow/tool_description_enhancer.go b/pkg/workflow/tool_description_enhancer.go index b0b7f102b3..47cb98f196 100644 --- a/pkg/workflow/tool_description_enhancer.go +++ b/pkg/workflow/tool_description_enhancer.go @@ -158,6 +158,13 @@ func enhanceToolDescription(toolName, baseDescription string, safeOutputs *SafeO } } + case "submit_pull_request_review": + if config := safeOutputs.SubmitPullRequestReview; config != nil { + if config.Max > 0 { + constraints = append(constraints, fmt.Sprintf("Maximum %d review(s) can be submitted.", config.Max)) + } + } + case "create_code_scanning_alert": if config := safeOutputs.CreateCodeScanningAlerts; config != nil { if config.Max > 0 { diff --git a/schemas/agent-output.json b/schemas/agent-output.json index e5e4df07b1..1de5c72e6e 100644 --- a/schemas/agent-output.json +++ b/schemas/agent-output.json @@ -52,7 +52,8 @@ { "$ref": "#/$defs/LinkSubIssueOutput" }, { "$ref": "#/$defs/HideCommentOutput" }, { "$ref": "#/$defs/DispatchWorkflowOutput" }, - { "$ref": "#/$defs/AutofixCodeScanningAlertOutput" } + { "$ref": "#/$defs/AutofixCodeScanningAlertOutput" }, + { "$ref": "#/$defs/SubmitPullRequestReviewOutput" } ] }, "CreateIssueOutput": { @@ -297,6 +298,47 @@ "required": ["type", "path", "line", "body"], "additionalProperties": false }, + "SubmitPullRequestReviewOutput": { + "title": "Submit Pull Request Review Output", + "description": "Output for submitting a PR review with a status (APPROVE, REQUEST_CHANGES, COMMENT). All create_pull_request_review_comment outputs are collected and submitted as part of this review. If omitted, comments are submitted as a COMMENT review.", + "type": "object", + "properties": { + "type": { + "const": "submit_pull_request_review" + }, + "body": { + "type": "string", + "description": "Overall review body text summarizing the review. Required and must be non-empty for APPROVE or REQUEST_CHANGES reviews; optional for COMMENT reviews." + }, + "event": { + "type": "string", + "description": "Review decision: APPROVE to approve the PR, REQUEST_CHANGES to request changes, or COMMENT for general feedback. Defaults to COMMENT when omitted.", + "enum": ["APPROVE", "REQUEST_CHANGES", "COMMENT"], + "default": "COMMENT" + } + }, + "required": ["type"], + "additionalProperties": false, + "allOf": [ + { + "if": { + "properties": { + "event": { "enum": ["APPROVE", "REQUEST_CHANGES"] } + }, + "required": ["event"] + }, + "then": { + "required": ["type", "event", "body"], + "properties": { + "body": { + "type": "string", + "minLength": 1 + } + } + } + } + ] + }, "CreateDiscussionOutput": { "title": "Create Discussion Output", "description": "Output for creating a GitHub discussion",