diff --git a/.github/workflows/ocr-review.yml b/.github/workflows/ocr-review.yml index 2a1ac38..ea4ffd5 100644 --- a/.github/workflows/ocr-review.yml +++ b/.github/workflows/ocr-review.yml @@ -136,7 +136,7 @@ jobs: owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, - body: `āš ļø **OpenCodeReview** encountered an error:\n\`\`\`\n${stderr}\n\`\`\`` + body: `āš ļø **OpenCodeReview** encountered an error:\n${fencedBlock(stderr)}` }); } return; @@ -208,27 +208,17 @@ jobs: reviewComment.side = 'RIGHT'; } - reviewComments.push(reviewComment); + reviewComments.push({ comment, reviewComment }); } // Submit as a single PR review with all comments const totalCount = comments.length; const inlineCount = reviewComments.length; const summaryCount = commentsWithoutLine.length; - let summaryBody = `šŸ” **OpenCodeReview** found **${totalCount}** issue(s) in this PR.`; - if (totalCount > 0) { - summaryBody += `\n- āœ… ${inlineCount} posted as inline comment(s)`; - summaryBody += `\n- šŸ“ ${summaryCount} posted as summary (missing line info)`; - } - if (warnings.length > 0) { - summaryBody += `\n\nāš ļø ${warnings.length} warning(s) occurred during review.`; - } + let summaryBody = buildSummaryBody(totalCount, inlineCount, summaryCount, warnings); // Add comments without line info to summary body - for (const { comment, body } of commentsWithoutLine) { - summaryBody += '\n\n---\n\n'; - summaryBody += formatCommentMarkdown(comment); - } + summaryBody += formatSummaryComments(commentsWithoutLine); // Statistics tracking let successCount = 0; @@ -243,7 +233,7 @@ jobs: commit_id: commitSha, body: summaryBody, event: 'COMMENT', - comments: reviewComments + comments: reviewComments.map(({ reviewComment }) => reviewComment) }); successCount = reviewComments.length; console.log(`Successfully posted review with ${successCount} inline comments (${commentsWithoutLine.length} in summary)`); @@ -252,7 +242,7 @@ jobs: console.log('Falling back to posting comments individually...'); // Fallback: post comments one by one - for (const reviewComment of reviewComments) { + for (const { comment, reviewComment } of reviewComments) { try { await github.rest.pulls.createReview({ owner: context.repo.owner, @@ -267,26 +257,27 @@ jobs: console.log(`Successfully posted comment for ${reviewComment.path}`); } catch (innerE) { failedCount++; - failedComments.push({ comment: reviewComment, error: innerE.message }); + failedComments.push({ comment, error: innerE.message }); console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`); } } // Post summary comment with statistics - let finalBody = summaryBody; + let finalBody = buildSummaryBody(totalCount, successCount, commentsWithoutLine.length + failedComments.length, warnings); + finalBody += formatSummaryComments(commentsWithoutLine); finalBody += `\n\n---\n\nšŸ“Š **Posting Statistics:**`; finalBody += `\n- āœ… Successfully posted: ${successCount} comment(s)`; if (failedCount > 0) { finalBody += `\n- āŒ Failed to post: ${failedCount} comment(s)`; } - // Add failed comments details + // Add failed comments as summary content so review feedback is not lost. if (failedComments.length > 0) { - finalBody += '\n\n
āŒ Failed Comments Details\n\n'; + finalBody += '\n\n---\n\n### āš ļø Inline comments shown in summary'; for (const { comment, error } of failedComments) { - finalBody += `- \`${comment.path}\`: ${error}\n`; + finalBody += '\n\n---\n\n'; + finalBody += formatCommentMarkdown(comment, error); } - finalBody += '\n
'; } await github.rest.issues.createComment({ @@ -303,29 +294,64 @@ jobs: // Add code suggestion if available if (comment.suggestion_code && comment.existing_code) { body += '\n\n**Suggestion:**\n'; - body += '```suggestion\n'; - body += comment.suggestion_code; - if (!comment.suggestion_code.endsWith('\n')) body += '\n'; - body += '```'; + body += fencedBlock(comment.suggestion_code, 'suggestion'); } return body; } - function formatCommentMarkdown(comment) { + function formatCommentMarkdown(comment, error) { let md = `### šŸ“„ \`${comment.path}\``; if (comment.start_line && comment.end_line) { md += ` (L${comment.start_line}-L${comment.end_line})`; } md += '\n\n'; + if (error) { + md += `āš ļø GitHub could not post this as an inline comment: ${error}\n\n`; + } md += comment.content || ''; if (comment.suggestion_code && comment.existing_code) { md += '\n\n
šŸ’” Suggested Change\n\n'; - md += '**Before:**\n```\n' + comment.existing_code + '\n```\n\n'; - md += '**After:**\n```\n' + comment.suggestion_code + '\n```\n\n'; + md += '**Before:**\n' + fencedBlock(comment.existing_code) + '\n\n'; + md += '**After:**\n' + fencedBlock(comment.suggestion_code) + '\n\n'; md += '
'; } return md; } + + function buildSummaryBody(totalCount, inlineCount, summaryCount, warnings) { + let body = `šŸ” **OpenCodeReview** found **${totalCount}** issue(s) in this PR.`; + if (totalCount > 0) { + body += `\n- āœ… ${inlineCount} posted as inline comment(s)`; + body += `\n- šŸ“ ${summaryCount} posted as summary`; + } + if (warnings.length > 0) { + body += `\n\nāš ļø ${warnings.length} warning(s) occurred during review.`; + } + return body; + } + + function formatSummaryComments(summaryComments) { + let body = ''; + for (const { comment } of summaryComments) { + body += '\n\n---\n\n'; + body += formatCommentMarkdown(comment); + } + return body; + } + + function fencedBlock(content, language = '') { + const text = String(content || ''); + const fence = safeFence(text); + let block = fence + language + '\n' + text; + if (!text.endsWith('\n')) block += '\n'; + return block + fence; + } + + function safeFence(content) { + const matches = String(content || '').match(/`+/g) || []; + const maxTicks = matches.reduce((max, ticks) => Math.max(max, ticks.length), 0); + return '`'.repeat(Math.max(3, maxTicks + 1)); + } diff --git a/examples/github_actions/ocr-review.yml b/examples/github_actions/ocr-review.yml index 793b354..ddc50c8 100644 --- a/examples/github_actions/ocr-review.yml +++ b/examples/github_actions/ocr-review.yml @@ -136,7 +136,7 @@ jobs: owner: context.repo.owner, repo: context.repo.repo, issue_number: context.issue.number, - body: `āš ļø **OpenCodeReview** encountered an error:\n\`\`\`\n${stderr}\n\`\`\`` + body: `āš ļø **OpenCodeReview** encountered an error:\n${fencedBlock(stderr)}` }); } return; @@ -208,27 +208,17 @@ jobs: reviewComment.side = 'RIGHT'; } - reviewComments.push(reviewComment); + reviewComments.push({ comment, reviewComment }); } // Submit as a single PR review with all comments const totalCount = comments.length; const inlineCount = reviewComments.length; const summaryCount = commentsWithoutLine.length; - let summaryBody = `šŸ” **OpenCodeReview** found **${totalCount}** issue(s) in this PR.`; - if (totalCount > 0) { - summaryBody += `\n- āœ… ${inlineCount} posted as inline comment(s)`; - summaryBody += `\n- šŸ“ ${summaryCount} posted as summary (missing line info)`; - } - if (warnings.length > 0) { - summaryBody += `\n\nāš ļø ${warnings.length} warning(s) occurred during review.`; - } + let summaryBody = buildSummaryBody(totalCount, inlineCount, summaryCount, warnings); // Add comments without line info to summary body - for (const { comment, body } of commentsWithoutLine) { - summaryBody += '\n\n---\n\n'; - summaryBody += formatCommentMarkdown(comment); - } + summaryBody += formatSummaryComments(commentsWithoutLine); // Statistics tracking let successCount = 0; @@ -243,7 +233,7 @@ jobs: commit_id: commitSha, body: summaryBody, event: 'COMMENT', - comments: reviewComments + comments: reviewComments.map(({ reviewComment }) => reviewComment) }); successCount = reviewComments.length; console.log(`Successfully posted review with ${successCount} inline comments (${commentsWithoutLine.length} in summary)`); @@ -252,7 +242,7 @@ jobs: console.log('Falling back to posting comments individually...'); // Fallback: post comments one by one - for (const reviewComment of reviewComments) { + for (const { comment, reviewComment } of reviewComments) { try { await github.rest.pulls.createReview({ owner: context.repo.owner, @@ -267,26 +257,27 @@ jobs: console.log(`Successfully posted comment for ${reviewComment.path}`); } catch (innerE) { failedCount++; - failedComments.push({ comment: reviewComment, error: innerE.message }); + failedComments.push({ comment, error: innerE.message }); console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`); } } // Post summary comment with statistics - let finalBody = summaryBody; + let finalBody = buildSummaryBody(totalCount, successCount, commentsWithoutLine.length + failedComments.length, warnings); + finalBody += formatSummaryComments(commentsWithoutLine); finalBody += `\n\n---\n\nšŸ“Š **Posting Statistics:**`; finalBody += `\n- āœ… Successfully posted: ${successCount} comment(s)`; if (failedCount > 0) { finalBody += `\n- āŒ Failed to post: ${failedCount} comment(s)`; } - // Add failed comments details + // Add failed comments as summary content so review feedback is not lost. if (failedComments.length > 0) { - finalBody += '\n\n
āŒ Failed Comments Details\n\n'; + finalBody += '\n\n---\n\n### āš ļø Inline comments shown in summary'; for (const { comment, error } of failedComments) { - finalBody += `- \`${comment.path}\`: ${error}\n`; + finalBody += '\n\n---\n\n'; + finalBody += formatCommentMarkdown(comment, error); } - finalBody += '\n
'; } await github.rest.issues.createComment({ @@ -303,29 +294,64 @@ jobs: // Add code suggestion if available if (comment.suggestion_code && comment.existing_code) { body += '\n\n**Suggestion:**\n'; - body += '```suggestion\n'; - body += comment.suggestion_code; - if (!comment.suggestion_code.endsWith('\n')) body += '\n'; - body += '```'; + body += fencedBlock(comment.suggestion_code, 'suggestion'); } return body; } - function formatCommentMarkdown(comment) { + function formatCommentMarkdown(comment, error) { let md = `### šŸ“„ \`${comment.path}\``; if (comment.start_line && comment.end_line) { md += ` (L${comment.start_line}-L${comment.end_line})`; } md += '\n\n'; + if (error) { + md += `āš ļø GitHub could not post this as an inline comment: ${error}\n\n`; + } md += comment.content || ''; if (comment.suggestion_code && comment.existing_code) { md += '\n\n
šŸ’” Suggested Change\n\n'; - md += '**Before:**\n```\n' + comment.existing_code + '\n```\n\n'; - md += '**After:**\n```\n' + comment.suggestion_code + '\n```\n\n'; + md += '**Before:**\n' + fencedBlock(comment.existing_code) + '\n\n'; + md += '**After:**\n' + fencedBlock(comment.suggestion_code) + '\n\n'; md += '
'; } return md; } + + function buildSummaryBody(totalCount, inlineCount, summaryCount, warnings) { + let body = `šŸ” **OpenCodeReview** found **${totalCount}** issue(s) in this PR.`; + if (totalCount > 0) { + body += `\n- āœ… ${inlineCount} posted as inline comment(s)`; + body += `\n- šŸ“ ${summaryCount} posted as summary`; + } + if (warnings.length > 0) { + body += `\n\nāš ļø ${warnings.length} warning(s) occurred during review.`; + } + return body; + } + + function formatSummaryComments(summaryComments) { + let body = ''; + for (const { comment } of summaryComments) { + body += '\n\n---\n\n'; + body += formatCommentMarkdown(comment); + } + return body; + } + + function fencedBlock(content, language = '') { + const text = String(content || ''); + const fence = safeFence(text); + let block = fence + language + '\n' + text; + if (!text.endsWith('\n')) block += '\n'; + return block + fence; + } + + function safeFence(content) { + const matches = String(content || '').match(/`+/g) || []; + const maxTicks = matches.reduce((max, ticks) => Math.max(max, ticks.length), 0); + return '`'.repeat(Math.max(3, maxTicks + 1)); + } diff --git a/package.json b/package.json index 5be4bb3..2809c16 100644 --- a/package.json +++ b/package.json @@ -6,7 +6,8 @@ "ocr": "bin/ocr.js" }, "scripts": { - "postinstall": "node scripts/install.js" + "postinstall": "node scripts/install.js", + "test:github-actions": "node scripts/github-actions/post-review-comments.test.js" }, "repository": { "type": "git", diff --git a/scripts/github-actions/post-review-comments.test.js b/scripts/github-actions/post-review-comments.test.js new file mode 100644 index 0000000..9fc1dcd --- /dev/null +++ b/scripts/github-actions/post-review-comments.test.js @@ -0,0 +1,171 @@ +#!/usr/bin/env node +"use strict"; + +const assert = require("assert"); +const fs = require("fs"); +const path = require("path"); +const vm = require("vm"); + +const repoRoot = path.join(__dirname, "..", ".."); +const workflowFiles = [ + ".github/workflows/ocr-review.yml", + "examples/github_actions/ocr-review.yml", +]; + +function extractPostReviewScript(workflowPath) { + const text = fs.readFileSync(path.join(repoRoot, workflowPath), "utf8"); + const lines = text.split("\n"); + + for (let i = 0; i < lines.length; i++) { + const line = lines[i]; + const marker = line.match(/^(\s*)script:\s*\|\s*$/); + if (!marker) continue; + + const blockIndent = marker[1].length + 2; + const block = []; + for (let j = i + 1; j < lines.length; j++) { + const current = lines[j]; + if (current.trim() === "") { + block.push(""); + continue; + } + const indent = current.match(/^ */)[0].length; + if (indent < blockIndent) break; + block.push(current.slice(blockIndent)); + } + + const script = block.join("\n"); + if (script.includes("/tmp/ocr-result.json")) { + return script; + } + } + + throw new Error(`post review script not found in ${workflowPath}`); +} + +function mockFs(resultText, stderrText) { + return { + readFileSync(file) { + if (file === "/tmp/ocr-result.json") return resultText; + if (file === "/tmp/ocr-stderr.log") return stderrText; + throw new Error(`unexpected read: ${file}`); + }, + }; +} + +function mockGithub(options) { + const createReviewCalls = []; + const issueComments = []; + + return { + createReviewCalls, + issueComments, + rest: { + pulls: { + get: async () => ({ data: { head: { sha: "head-sha" } } }), + createReview: async (params) => { + createReviewCalls.push(params); + if (createReviewCalls.length === 1 && options.bulkError) { + throw new Error(options.bulkError); + } + if (createReviewCalls.length > 1 && options.individualError) { + throw new Error(options.individualError); + } + return { data: {} }; + }, + }, + issues: { + createComment: async (params) => { + issueComments.push(params); + return { data: {} }; + }, + }, + }, + }; +} + +async function runPostReviewScript(workflowPath, options) { + const script = extractPostReviewScript(workflowPath); + const github = mockGithub(options); + const context = { + repo: { owner: "owner", repo: "repo" }, + issue: { number: 123 }, + eventName: "pull_request_target", + payload: { pull_request: { head: { sha: "head-sha" } } }, + }; + const sandbox = { + github, + context, + console: { log() {} }, + require(name) { + if (name === "fs") return options.fs; + throw new Error(`unexpected require: ${name}`); + }, + }; + + await vm.runInNewContext(`(async () => {\n${script}\n})()`, sandbox, { + timeout: 1000, + }); + + return github; +} + +async function testFailedInlineCommentsAreSummarized(workflowPath) { + const result = { + comments: [ + { + path: "docs/no-line.md", + content: + "No-line content with a fenced block:\n\n```js\nconsole.log('still visible');\n```", + existing_code: "", + suggestion_code: "", + start_line: 0, + end_line: 0, + }, + { + path: "src/app.js", + content: "Failed inline content must remain visible in the PR summary.", + existing_code: "oldCall();", + suggestion_code: "newCall();", + start_line: 10, + end_line: 10, + }, + ], + warnings: [], + }; + + const github = await runPostReviewScript(workflowPath, { + fs: mockFs(JSON.stringify(result), ""), + bulkError: 'Unprocessable Entity: "Line could not be resolved"', + individualError: 'Unprocessable Entity: "Line could not be resolved"', + }); + + assert.strictEqual(github.createReviewCalls.length, 2); + assert.strictEqual(github.issueComments.length, 1); + const body = github.issueComments[0].body; + assert.match(body, /No-line content with a fenced block/); + assert.match(body, /Failed inline content must remain visible/); + assert.match(body, /Line could not be resolved/); +} + +async function testErrorCommentUsesSafeFence(workflowPath) { + const github = await runPostReviewScript(workflowPath, { + fs: mockFs("not json", "stderr includes a fence\n```js\nbroken();\n```"), + }); + + assert.strictEqual(github.issueComments.length, 1); + const body = github.issueComments[0].body; + assert.match(body, /\n````\nstderr includes a fence/); +} + +async function main() { + for (const workflowPath of workflowFiles) { + await testFailedInlineCommentsAreSummarized(workflowPath); + await testErrorCommentUsesSafeFence(workflowPath); + } +} + +main().catch((err) => { + console.error(err); + process.exit(1); +});