From 600162b09bfd6f54a1bb491d4524ee036da66771 Mon Sep 17 00:00:00 2001 From: Tosin Afolabi Date: Fri, 13 Feb 2026 09:54:52 -0500 Subject: [PATCH] refactor: align sticky comment isolation with PR #780 patterns - Use `` header format instead of `` - Add extractBotHeader() for cleaner header extraction - Move header preservation to MCP server instead of modifying sanitizer - Adopt regex-based matching with hasAnyHeader check and legacy fallback - Use context.inputs.botId for dynamic bot ID matching - Revert sanitizer.ts to keep security code untouched Co-Authored-By: Claude Opus 4.6 --- src/github/operations/comment-logic.ts | 23 ++++---- src/github/operations/comments/common.ts | 17 +++--- .../operations/comments/create-initial.ts | 54 +++++++++++++----- .../operations/comments/update-with-branch.ts | 9 ++- src/github/utils/sanitizer.ts | 7 +-- src/mcp/github-comment-server.ts | 21 ++++++- test/comment-logic.test.ts | 55 +++++++++++++++++-- test/sanitizer.test.ts | 23 +------- 8 files changed, 137 insertions(+), 72 deletions(-) diff --git a/src/github/operations/comment-logic.ts b/src/github/operations/comment-logic.ts index b4f5cbf..fcd9e9b 100644 --- a/src/github/operations/comment-logic.ts +++ b/src/github/operations/comment-logic.ts @@ -1,4 +1,5 @@ import { GITHUB_SERVER_URL } from "../api/config"; +import { extractBotHeader } from "./comments/common"; export type ExecutionDetails = { total_cost_usd?: number; @@ -79,21 +80,18 @@ export function updateCommentBody(input: CommentUpdateInput): string { errorDetails, } = input; - // Preserve sticky-job header if present - const stickyHeaderMatch = originalBody.match( - /^()\n?/, - ); - const stickyHeader = stickyHeaderMatch ? stickyHeaderMatch[1] + "\n" : ""; + // Extract and preserve bot header for sticky comment identification + const botHeader = extractBotHeader(originalBody); // Extract content from the original comment body // First, remove the "Claude Code is working…" or "Claude Code is working..." message const workingPattern = /Claude Code is working[…\.]{1,3}(?:\s*]*>)?/i; let bodyContent = originalBody.replace(workingPattern, "").trim(); - // Remove sticky-job header from body content (it's re-prepended separately) - bodyContent = bodyContent - .replace(/^\n?/, "") - .trim(); + // Remove bot header from body content since we'll prepend it at the end + if (botHeader) { + bodyContent = bodyContent.replace(/^\n?/, "").trim(); + } // Check if there's a PR link in the content let prLinkFromContent = ""; @@ -190,7 +188,7 @@ export function updateCommentBody(input: CommentUpdateInput): string { } // Build the new body with blank line between header and separator - let newBody = `${stickyHeader}${header}${links}`; + let newBody = `${header}${links}`; // Add error details if available if (actionFailed && errorDetails) { @@ -210,5 +208,10 @@ export function updateCommentBody(input: CommentUpdateInput): string { // Add the cleaned body content newBody += bodyContent; + // Prepend bot header if it existed in the original comment + if (botHeader) { + return (botHeader + newBody).trim(); + } + return newBody.trim(); } diff --git a/src/github/operations/comments/common.ts b/src/github/operations/comments/common.ts index 5b9d39c..c29861f 100644 --- a/src/github/operations/comments/common.ts +++ b/src/github/operations/comments/common.ts @@ -21,23 +21,20 @@ export function createBranchLink( return `\n[View branch](${branchUrl})`; } -export function createStickyJobHeader(jobId: string): string { - return ``; -} - -export function hasStickyJobHeader(body: string, jobId: string): boolean { - return body.includes(createStickyJobHeader(jobId)); -} - export function createCommentBody( jobRunLink: string, branchLink: string = "", - jobId: string = "", + botName: string = "", ): string { - const header = jobId ? `${createStickyJobHeader(jobId)}\n` : ""; + const header = botName ? `\n` : ""; return `${header}Claude Code is working… ${SPINNER_HTML} I'll analyze this and get back to you. ${jobRunLink}${branchLink}`; } + +export function extractBotHeader(body: string): string { + const match = body.match(/^(\n?)/); + return match?.[1] ?? ""; +} diff --git a/src/github/operations/comments/create-initial.ts b/src/github/operations/comments/create-initial.ts index 67efdca..a7fb4bd 100644 --- a/src/github/operations/comments/create-initial.ts +++ b/src/github/operations/comments/create-initial.ts @@ -6,28 +6,29 @@ */ import { appendFileSync } from "fs"; -import { - createJobRunLink, - createCommentBody, - hasStickyJobHeader, -} from "./common"; +import { createJobRunLink, createCommentBody } from "./common"; import { isPullRequestReviewCommentEvent, isPullRequestEvent, type ParsedGitHubContext, } from "../../context"; -import { CLAUDE_APP_BOT_ID } from "../../constants"; import type { Octokit } from "@octokit/rest"; +function escapeRegex(str: string): string { + return str.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + export async function createInitialComment( octokit: Octokit, context: ParsedGitHubContext, ) { const { owner, repo } = context.repository; - const jobId = context.inputs.jobId; + const botIdentifier = context.inputs.useStickyComment + ? context.inputs.jobId + : ""; const jobRunLink = createJobRunLink(owner, repo, context.runId); - const initialBody = createCommentBody(jobRunLink, "", jobId); + const initialBody = createCommentBody(jobRunLink, "", botIdentifier); try { let response; @@ -37,6 +38,7 @@ export async function createInitialComment( context.isPR && isPullRequestEvent(context) ) { + // Use pagination to fetch all comments (handles PRs with 30+ comments) const comments = await octokit.paginate( octokit.rest.issues.listComments, { @@ -48,15 +50,41 @@ export async function createInitialComment( ); const existingComment = comments.find((comment) => { - if (jobId) { - return hasStickyJobHeader(comment.body ?? "", jobId); + // Must be from the correct bot user + const idMatch = comment.user?.id === Number(context.inputs.botId); + if (!idMatch) return false; + + if (botIdentifier) { + // Check for our hidden header (case-insensitive, whitespace-tolerant) + const headerPattern = new RegExp( + ``, + "i", + ); + const hasOurHeader = headerPattern.test(comment.body || ""); + + // Check if comment has ANY bot header + const hasAnyHeader = //.test( + comment.body || "", + ); + + // If comment has a header, ONLY match if it's ours + if (hasAnyHeader) { + return hasOurHeader; + } + + // Legacy comments (no header): match if it looks like a Claude comment + const isClaudeComment = + comment.body?.includes("Claude Code is working") || + comment.body?.includes("View job run"); + + return isClaudeComment; } - // Fallback for backward compat when no jobId is available - const idMatch = comment.user?.id === CLAUDE_APP_BOT_ID; + + // Fallback when no botIdentifier: match any Claude-looking comment const botNameMatch = comment.user?.type === "Bot" && comment.user?.login.toLowerCase().includes("claude"); - return idMatch || botNameMatch; + return botNameMatch; }); if (existingComment) { diff --git a/src/github/operations/comments/update-with-branch.ts b/src/github/operations/comments/update-with-branch.ts index 0df40c9..7de280a 100644 --- a/src/github/operations/comments/update-with-branch.ts +++ b/src/github/operations/comments/update-with-branch.ts @@ -33,11 +33,10 @@ export async function updateTrackingComment( branchLink = createBranchLink(owner, repo, branch); } - const updatedBody = createCommentBody( - jobRunLink, - branchLink, - context.inputs.jobId, - ); + const botIdentifier = context.inputs.useStickyComment + ? context.inputs.jobId + : ""; + const updatedBody = createCommentBody(jobRunLink, branchLink, botIdentifier); // Update the existing comment with the branch link try { diff --git a/src/github/utils/sanitizer.ts b/src/github/utils/sanitizer.ts index fd5d8a6..83ee096 100644 --- a/src/github/utils/sanitizer.ts +++ b/src/github/utils/sanitizer.ts @@ -97,9 +97,4 @@ export function redactGitHubTokens(content: string): string { } export const stripHtmlComments = (content: string) => - content.replace(//g, (match) => { - if (match.startsWith("/g, ""); diff --git a/src/mcp/github-comment-server.ts b/src/mcp/github-comment-server.ts index ef6728c..d699938 100644 --- a/src/mcp/github-comment-server.ts +++ b/src/mcp/github-comment-server.ts @@ -5,6 +5,7 @@ import { StdioServerTransport } from "@modelcontextprotocol/sdk/server/stdio.js" import { z } from "zod"; import { GITHUB_API_URL } from "../github/api/config"; import { Octokit } from "@octokit/rest"; +import { extractBotHeader } from "../github/operations/comments/common"; import { updateClaudeComment } from "../github/operations/comments/update-claude-comment"; import { sanitizeContent } from "../github/utils/sanitizer"; @@ -55,13 +56,31 @@ server.tool( const isPullRequestReviewComment = eventName === "pull_request_review_comment"; + // Fetch current comment to preserve the bot header for sticky comment functionality + const currentComment = isPullRequestReviewComment + ? await octokit.rest.pulls.getReviewComment({ + owner, + repo, + comment_id: commentId, + }) + : await octokit.rest.issues.getComment({ + owner, + repo, + comment_id: commentId, + }); + + // Extract bot header if present (e.g., "") + const botHeader = extractBotHeader(currentComment.data.body || ""); + + // Sanitize then prepend header to preserve sticky comment identification const sanitizedBody = sanitizeContent(body); + const bodyWithHeader = botHeader + sanitizedBody; const result = await updateClaudeComment(octokit, { owner, repo, commentId, - body: sanitizedBody, + body: bodyWithHeader, isPullRequestReviewComment, }); diff --git a/test/comment-logic.test.ts b/test/comment-logic.test.ts index de915d2..3520b5b 100644 --- a/test/comment-logic.test.ts +++ b/test/comment-logic.test.ts @@ -444,11 +444,11 @@ describe("updateCommentBody", () => { }); }); - describe("sticky-job header preservation", () => { - it("should preserve sticky-job header through comment update", () => { + describe("bot header preservation", () => { + it("should preserve bot header through comment update", () => { const input: CommentUpdateInput = { currentBody: - "\nClaude Code is working…\n\nI'll analyze this and get back to you.\n\n[View job run](https://github.com/owner/repo/actions/runs/123)", + "\nClaude Code is working…\n\nI'll analyze this and get back to you.\n\n[View job run](https://github.com/owner/repo/actions/runs/123)", actionFailed: false, executionDetails: { duration_ms: 30000 }, jobUrl: "https://github.com/owner/repo/actions/runs/123", @@ -456,11 +456,25 @@ describe("updateCommentBody", () => { }; const result = updateCommentBody(input); - expect(result).toStartWith("\n"); + expect(result).toStartWith("\n"); expect(result).toContain("Claude finished @test-user's task"); }); - it("should work without sticky-job header", () => { + it("should preserve bot header with different bot names", () => { + const input: CommentUpdateInput = { + currentBody: + "\nClaude Code is working...", + actionFailed: false, + executionDetails: { duration_ms: 30000 }, + jobUrl: "https://github.com/owner/repo/actions/runs/123", + triggerUsername: "user", + }; + + const result = updateCommentBody(input); + expect(result).toStartWith(""); + }); + + it("should work without bot header", () => { const input: CommentUpdateInput = { currentBody: "Claude Code is working…\n\nI'll analyze this and get back to you.", @@ -471,8 +485,37 @@ describe("updateCommentBody", () => { }; const result = updateCommentBody(input); - expect(result).not.toContain("sticky-job"); + expect(result).not.toContain("\nClaude Code is working…", + actionFailed: true, + executionDetails: { duration_ms: 10000 }, + jobUrl: "https://github.com/owner/repo/actions/runs/123", + errorDetails: "Something went wrong", + }; + + const result = updateCommentBody(input); + expect(result).toStartWith(""); + expect(result).toContain("**Claude encountered an error after 10s**"); + }); + + it("should not preserve bot header if not at start of comment", () => { + const input: CommentUpdateInput = { + currentBody: + "Some text before\n\nClaude Code is working…", + actionFailed: false, + executionDetails: { duration_ms: 5000 }, + jobUrl: "https://github.com/owner/repo/actions/runs/123", + triggerUsername: "testuser", + }; + + const result = updateCommentBody(input); + expect(result.startsWith("Text")).toBe( - "Text", - ); - }); - - it("should preserve sticky-job headers while stripping other comments", () => { - const input = - "\nHello World"; - expect(stripHtmlComments(input)).toBe( - "\nHello World", - ); - }); -}); - -describe("sanitizeContent with sticky-job headers", () => { - it("should preserve sticky-job headers through full sanitization", () => { - const content = "\nSome response text"; - const sanitized = sanitizeContent(content); - expect(sanitized).toContain(""); - expect(sanitized).toContain("Some response text"); + it("should strip all HTML comments including bot headers", () => { + expect(stripHtmlComments("Text")).toBe("Text"); }); });