forked from claude-did-this/claude-hub
Compare commits
1 Commits
main
...
fix/pr-rev
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
23747050cb |
@@ -1375,6 +1375,31 @@ Create a single comprehensive review comment with all feedback.
|
||||
6. **Complete your review** with an appropriate event type (APPROVE, REQUEST_CHANGES, or COMMENT)
|
||||
7. **Include commit SHA** - Always include "Reviewed at commit: ${commitSha}" in your final review comment
|
||||
|
||||
## CRITICAL: Markdown Formatting Requirements
|
||||
|
||||
**IMPORTANT**: When writing your review comments and responses:
|
||||
- Return clean, human-readable markdown that GitHub will render correctly
|
||||
- Do NOT escape or encode special characters like newlines (\\n), quotes, or backslashes
|
||||
- Use proper markdown syntax directly:
|
||||
- Use actual bullet points: * or - (not escaped versions)
|
||||
- Use actual code blocks with triple backticks (not escaped)
|
||||
- Use actual line breaks (press Enter, don't write \\n)
|
||||
- Use actual quotes " or ' (not escaped versions like \\" or \\')
|
||||
- Your output should look like normal markdown text, NOT escaped strings
|
||||
- GitHub expects standard markdown - write it naturally as you would in any markdown editor
|
||||
|
||||
Example of CORRECT formatting:
|
||||
* This is a bullet point
|
||||
* Another bullet point with \`inline code\`
|
||||
|
||||
\`\`\`javascript
|
||||
// This is a code block
|
||||
const example = "string";
|
||||
\`\`\`
|
||||
|
||||
Example of INCORRECT formatting (DO NOT DO THIS):
|
||||
\\* This is a bullet point\\n\\* Another bullet point with \\\`inline code\\\`\\n\\n\\\`\\\`\\\`javascript\\n// This is a code block\\nconst example = \\"string\\";\\n\\\`\\\`\\\`
|
||||
|
||||
Please perform a comprehensive review of PR #${prNumber} in repository ${repoFullName}.`;
|
||||
}
|
||||
|
||||
|
||||
@@ -3,7 +3,7 @@ import { promisify } from 'util';
|
||||
import { execFile } from 'child_process';
|
||||
import path from 'path';
|
||||
import { createLogger } from '../utils/logger';
|
||||
import { sanitizeBotMentions } from '../utils/sanitize';
|
||||
import { sanitizeBotMentions, unescapeMarkdown } from '../utils/sanitize';
|
||||
import secureCredentials from '../utils/secureCredentials';
|
||||
import type {
|
||||
ClaudeCommandOptions,
|
||||
@@ -79,7 +79,8 @@ Since this is a test environment, I'm providing a simulated response. In product
|
||||
For real functionality, please configure valid GitHub and Claude API tokens.`;
|
||||
|
||||
// Always sanitize responses, even in test mode
|
||||
return sanitizeBotMentions(testResponse);
|
||||
const unescapedResponse = unescapeMarkdown(testResponse);
|
||||
return sanitizeBotMentions(unescapedResponse);
|
||||
}
|
||||
|
||||
// Build Docker image if it doesn't exist
|
||||
@@ -195,6 +196,9 @@ For real functionality, please configure valid GitHub and Claude API tokens.`;
|
||||
}
|
||||
}
|
||||
|
||||
// Unescape any markdown formatting that may have been escaped by Claude
|
||||
responseText = unescapeMarkdown(responseText);
|
||||
|
||||
// Sanitize response to prevent infinite loops by removing bot mentions
|
||||
responseText = sanitizeBotMentions(responseText);
|
||||
|
||||
|
||||
@@ -101,3 +101,43 @@ export function sanitizeEnvironmentValue(key: string, value: string): string {
|
||||
|
||||
return isSensitive ? '[REDACTED]' : value;
|
||||
}
|
||||
|
||||
/**
|
||||
* Unescapes markdown formatting that may have been escaped by Claude
|
||||
* This is a fallback for when Claude escapes markdown despite instructions
|
||||
*/
|
||||
export function unescapeMarkdown(text: string): string {
|
||||
if (!text) return text;
|
||||
|
||||
// Replace escaped newlines with actual newlines
|
||||
let unescaped = text.replace(/\\n/g, '\n');
|
||||
|
||||
// Replace escaped quotes
|
||||
unescaped = unescaped.replace(/\\"/g, '"');
|
||||
unescaped = unescaped.replace(/\\'/g, "'");
|
||||
|
||||
// Replace escaped backticks (for code blocks)
|
||||
unescaped = unescaped.replace(/\\`/g, '`');
|
||||
|
||||
// Replace escaped backslashes (but be careful not to double-unescape)
|
||||
// Only replace \\ with \ if it's not followed by another escape sequence
|
||||
unescaped = unescaped.replace(/\\\\(?![nrt"`'])/g, '\\');
|
||||
|
||||
// Replace escaped asterisks and other markdown characters
|
||||
unescaped = unescaped.replace(/\\\*/g, '*');
|
||||
unescaped = unescaped.replace(/\\_/g, '_');
|
||||
unescaped = unescaped.replace(/\\-/g, '-');
|
||||
unescaped = unescaped.replace(/\\#/g, '#');
|
||||
unescaped = unescaped.replace(/\\>/g, '>');
|
||||
unescaped = unescaped.replace(/\\\[/g, '[');
|
||||
unescaped = unescaped.replace(/\\\]/g, ']');
|
||||
unescaped = unescaped.replace(/\\\(/g, '(');
|
||||
unescaped = unescaped.replace(/\\\)/g, ')');
|
||||
|
||||
// Log if unescaping occurred
|
||||
if (unescaped !== text) {
|
||||
logger.info('Unescaped markdown formatting in text');
|
||||
}
|
||||
|
||||
return unescaped;
|
||||
}
|
||||
|
||||
@@ -4,7 +4,8 @@ import {
|
||||
sanitizeCommandInput,
|
||||
validateRepositoryName,
|
||||
validateGitHubRef,
|
||||
sanitizeEnvironmentValue
|
||||
sanitizeEnvironmentValue,
|
||||
unescapeMarkdown
|
||||
} from '../../../src/utils/sanitize';
|
||||
|
||||
describe('Sanitize Utils', () => {
|
||||
@@ -179,4 +180,56 @@ describe('Sanitize Utils', () => {
|
||||
expect(sanitizeEnvironmentValue('DB_PASSWORD_HASH', 'value')).toBe('[REDACTED]');
|
||||
});
|
||||
});
|
||||
|
||||
describe('unescapeMarkdown', () => {
|
||||
it('should unescape escaped newlines', () => {
|
||||
const input = 'Line 1\\nLine 2\\nLine 3';
|
||||
const expected = 'Line 1\nLine 2\nLine 3';
|
||||
expect(unescapeMarkdown(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should unescape escaped quotes', () => {
|
||||
const input = 'This is a \\"quoted\\" string and this is a \\\'single quoted\\\' string';
|
||||
const expected = 'This is a "quoted" string and this is a \'single quoted\' string';
|
||||
expect(unescapeMarkdown(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should unescape markdown characters', () => {
|
||||
const input = '\\* This is a bullet\\n\\- Another bullet\\n\\# Header\\n\\`code\\`';
|
||||
const expected = '* This is a bullet\n- Another bullet\n# Header\n`code`';
|
||||
expect(unescapeMarkdown(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should handle escaped code blocks', () => {
|
||||
const input = '\\`\\`\\`javascript\\nconst x = \\"test\\";\\n\\`\\`\\`';
|
||||
const expected = '```javascript\nconst x = "test";\n```';
|
||||
expect(unescapeMarkdown(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should handle mixed markdown escaping', () => {
|
||||
const input =
|
||||
'\\# PR Review\\n\\n\\* Issue: Code has problems\\n\\* Suggestion: Fix them\\n\\n\\`\\`\\`js\\ncode()\\n\\`\\`\\`';
|
||||
const expected =
|
||||
'# PR Review\n\n* Issue: Code has problems\n* Suggestion: Fix them\n\n```js\ncode()\n```';
|
||||
expect(unescapeMarkdown(input)).toBe(expected);
|
||||
});
|
||||
|
||||
it('should handle empty or null input', () => {
|
||||
expect(unescapeMarkdown('')).toBe('');
|
||||
expect(unescapeMarkdown(null as any)).toBe(null);
|
||||
expect(unescapeMarkdown(undefined as any)).toBe(undefined);
|
||||
});
|
||||
|
||||
it('should not modify already correct markdown', () => {
|
||||
const input =
|
||||
'# Header\n\n* Bullet point\n* Another bullet\n\n```javascript\nconst x = "test";\n```';
|
||||
expect(unescapeMarkdown(input)).toBe(input);
|
||||
});
|
||||
|
||||
it('should handle escaped brackets and parentheses', () => {
|
||||
const input = 'Link: \\[text\\]\\(url\\) and another \\[link\\]';
|
||||
const expected = 'Link: [text](url) and another [link]';
|
||||
expect(unescapeMarkdown(input)).toBe(expected);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user