refactor: align sticky comment isolation with PR #780 patterns
- Use `<!-- bot: {id} -->` header format instead of `<!-- sticky-job: -->`
- 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 <noreply@anthropic.com>
This commit is contained in:
parent
263b3b725a
commit
600162b09b
@ -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(
|
||||
/^(<!-- sticky-job: [^\n]+ -->)\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*<img[^>]*>)?/i;
|
||||
let bodyContent = originalBody.replace(workingPattern, "").trim();
|
||||
|
||||
// Remove sticky-job header from body content (it's re-prepended separately)
|
||||
bodyContent = bodyContent
|
||||
.replace(/^<!-- sticky-job: [^\n]+ -->\n?/, "")
|
||||
.trim();
|
||||
// Remove bot header from body content since we'll prepend it at the end
|
||||
if (botHeader) {
|
||||
bodyContent = bodyContent.replace(/^<!--\s*bot:\s*\S+\s*-->\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();
|
||||
}
|
||||
|
||||
@ -21,23 +21,20 @@ export function createBranchLink(
|
||||
return `\n[View branch](${branchUrl})`;
|
||||
}
|
||||
|
||||
export function createStickyJobHeader(jobId: string): string {
|
||||
return `<!-- sticky-job: ${jobId} -->`;
|
||||
}
|
||||
|
||||
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 ? `<!-- bot: ${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(/^(<!--\s*bot:\s*\S+\s*-->\n?)/);
|
||||
return match?.[1] ?? "";
|
||||
}
|
||||
|
||||
@ -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(
|
||||
`<!--\\s*bot:\\s*${escapeRegex(botIdentifier)}\\s*-->`,
|
||||
"i",
|
||||
);
|
||||
const hasOurHeader = headerPattern.test(comment.body || "");
|
||||
|
||||
// Check if comment has ANY bot header
|
||||
const hasAnyHeader = /<!--\s*bot:\s*\S+\s*-->/.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) {
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -97,9 +97,4 @@ export function redactGitHubTokens(content: string): string {
|
||||
}
|
||||
|
||||
export const stripHtmlComments = (content: string) =>
|
||||
content.replace(/<!--[\s\S]*?-->/g, (match) => {
|
||||
if (match.startsWith("<!-- sticky-job:")) {
|
||||
return match;
|
||||
}
|
||||
return "";
|
||||
});
|
||||
content.replace(/<!--[\s\S]*?-->/g, "");
|
||||
|
||||
@ -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., "<!-- bot: claude-code-review -->")
|
||||
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,
|
||||
});
|
||||
|
||||
|
||||
@ -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:
|
||||
"<!-- sticky-job: claude-review -->\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)",
|
||||
"<!-- bot: claude-review -->\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("<!-- sticky-job: claude-review -->\n");
|
||||
expect(result).toStartWith("<!-- bot: claude-review -->\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:
|
||||
"<!-- bot: claude-docs-review -->\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("<!-- bot: claude-docs-review -->");
|
||||
});
|
||||
|
||||
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("<!-- bot:");
|
||||
expect(result).toContain("Claude finished @test-user's task");
|
||||
});
|
||||
|
||||
it("should preserve bot header when action fails", () => {
|
||||
const input: CommentUpdateInput = {
|
||||
currentBody: "<!-- bot: claude-review -->\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("<!-- bot: claude-review -->");
|
||||
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<!-- bot: claude-review -->\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("<!-- bot:")).toBe(false);
|
||||
expect(result).toContain("**Claude finished");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@ -361,26 +361,7 @@ describe("stripHtmlComments (legacy)", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("should preserve sticky-job headers", () => {
|
||||
expect(stripHtmlComments("<!-- sticky-job: claude-review -->Text")).toBe(
|
||||
"<!-- sticky-job: claude-review -->Text",
|
||||
);
|
||||
});
|
||||
|
||||
it("should preserve sticky-job headers while stripping other comments", () => {
|
||||
const input =
|
||||
"<!-- sticky-job: my-job -->\n<!-- hidden prompt -->Hello World";
|
||||
expect(stripHtmlComments(input)).toBe(
|
||||
"<!-- sticky-job: my-job -->\nHello World",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("sanitizeContent with sticky-job headers", () => {
|
||||
it("should preserve sticky-job headers through full sanitization", () => {
|
||||
const content = "<!-- sticky-job: claude-review -->\nSome response text";
|
||||
const sanitized = sanitizeContent(content);
|
||||
expect(sanitized).toContain("<!-- sticky-job: claude-review -->");
|
||||
expect(sanitized).toContain("Some response text");
|
||||
it("should strip all HTML comments including bot headers", () => {
|
||||
expect(stripHtmlComments("<!-- bot: claude-review -->Text")).toBe("Text");
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user