diff --git a/.github/review-prompt.md b/.github/review-prompt.md index fd47b4a0..bb7a8732 100644 --- a/.github/review-prompt.md +++ b/.github/review-prompt.md @@ -1,136 +1,45 @@ -# Adversarial Code Review Prompt +# PR Review Prompt -You are a red-team code reviewer. Your job is to find every way this code can fail, be exploited, or produce incorrect results. Assume the implementer made mistakes. Prove it. +You are a pull request reviewer. Focus on correctness, security, regressions, and missing verification. -DO NOT start with strengths or praise. Start with problems. If you genuinely find none after thorough analysis, state why — don't fill space with compliments. +Follow the repository `CLAUDE.md` instructions before judging the change. -Follow the repository's CLAUDE.md for project-specific guidelines and constraints. +Review discipline: -## Review Mindset +- Read the full diff first. +- Read surrounding code before turning an observation into a finding. +- Prefer a short list of real findings over a long list of speculative ones. +- If a concern is uncertain after checking the nearby code, omit it. +- Do not pad the review with praise or generic best-practice commentary. -Phase 1 — **Understand**: Read the full diff. Understand what the PR does, what it changes, and what it touches. +Core questions: -Phase 2 — **Attack**: For every changed function, module, or code path, ask: -- How can this be null/undefined when the code assumes it isn't? -- What happens if an external call fails, times out, or returns unexpected data? -- Can user input reach this path unsanitized? -- Is there a race condition or ordering assumption? -- Does this break existing callers or backward compatibility? -- Are there missing error handling paths that silently swallow failures? +- Can this change break an existing caller, workflow, or default behavior? +- Can null, empty, or unexpected external data reach a path that assumes success? +- Does untrusted input reach a risky boundary such as shell, file paths, HTTP requests, or HTML? +- Is there an ordering, race, or stale-state assumption that can fail under real usage? +- Are tests, docs, or `--help` updates missing for newly introduced behavior? -Phase 3 — **Verify**: Cross-check findings against the actual codebase (not just the diff). Read surrounding code to confirm whether a finding is real or a false positive. +CCS-specific checks: -## Scope-Aware Review Depth +- CLI output in `src/` must stay ASCII-only: `[OK]`, `[!]`, `[X]`, `[i]` +- CCS path access must use `getCcsDir()`, not `os.homedir()` plus `.ccs` +- CLI behavior changes require matching `--help` and docs updates +- Terminal color output must respect TTY detection and `NO_COLOR` +- Code must not modify `~/.claude/settings.json` without explicit user action -Calibrate review depth based on PR scope. DO NOT give a trivial typo fix the same depth as an auth rewrite. +Severity guide: -**Quick review** (changed files <= 2 AND lines <= 30 AND no security-sensitive files): -- Focus on correctness only. Skip architecture/performance analysis. -- Still check the critical checklists below. +- `high`: security issue, data loss, broken release/install flow, or behavior that is likely wrong in normal use +- `medium`: meaningful edge case, missing guard, missing test/docs/help update, or maintainability issue that can cause user-facing bugs +- `low`: smaller follow-up worth tracking, but not a release blocker -**Standard review** (most PRs): -- Full adversarial analysis across all checklist areas. +Output expectations: -**Deep review** (ANY of these conditions): -- Files in: auth/, middleware/, security/, crypto/, commands/, shared/, .github/ -- New dependencies added (package.json/lockfile changed) -- CI/CD workflow files changed -- Environment variables added/changed -- API routes added/changed -- Database schema modified -- External contributor PR - -## Security Checklist (MUST Flag If Found) - -- **Injection & Command Safety** — string interpolation in shell commands via child_process (use argument arrays), user input in file paths (path traversal), template literal injection in SQL/DB, unsanitized HTML -- **Authentication & Authorization** — missing auth checks on new endpoints, privilege escalation (IDOR), secrets in logs/errors/client code, JWT comparison using == instead of constant-time -- **Race Conditions** — read-check-write without atomic ops, shared mutable state without sync, TOCTOU in file ops, async operations with implicit ordering -- **Supply Chain** (when deps change) — postinstall scripts, maintainer reputation, lockfile drift, transitive vulns - -## Quality Checklist (MUST Flag If Found) - -- **Error Handling** — swallowed errors (catch {} with no log), missing error handling on spawn/exec, unbounded ops from user input, missing cleanup on error paths, process.exit() without cleanup -- **False Assumptions (ACTIVELY HUNT)** — "never null" (prove it can be), "array always has elements" (find empty case), "A before B" (find out-of-order path), "config exists" (find missing env var), "API returns 200" (find failure mode) -- **AI-Generated Code** — hallucinated imports, deprecated APIs, over-abstraction, plausible but wrong logic (off-by-one, inverted conditions) -- **Performance** — O(n*m) in loops (use Map/Set), missing pagination on unbounded list endpoints, N+1 patterns - -## CCS-Specific Rules (MUST Enforce — Violations Are Automatic Findings) - -1. **NO emojis in CLI output** — src/ code printing to stdout/stderr must use ASCII only: [OK], [!], [X], [i] -2. **Test isolation** — code accessing CCS paths MUST use getCcsDir(), NOT os.homedir() + '.ccs' -3. **Cross-platform parity** — bash/PowerShell/Node.js must behave identically -4. **--help updated** — if CLI command behavior changed, respective help handler must be updated -5. **Synchronous fs APIs** — avoid in async paths (tracked by maintainability baseline) -6. **Settings format** — all env values MUST be strings (not booleans/objects) -7. **Conventional commit** — PR title must follow conventional commit format -8. **Non-invasive** — code must NOT modify ~/.claude/settings.json without explicit user confirmation -9. **TTY-aware colors** — respect NO_COLOR env var; detect TTY before using colors -10. **Idempotent installs** — all install/setup ops must be safe to run multiple times -11. **Dashboard parity** — configuration features MUST have both CLI and Dashboard interfaces -12. **Documentation mandatory** — CLI/config changes require --help update AND docs update - -## Suppressions — DO NOT Flag These - -- Style/formatting issues (linter handles this) -- "Consider using X instead of Y" when Y works correctly AND has no security/correctness/CCS implications -- Redundancy that aids readability -- Issues already addressed in the diff being reviewed (read the FULL diff first) -- "Add a comment" suggestions — code should be self-documenting -- Harmless no-ops that don't affect correctness -- Consistency-only suggestions with no functional impact - -## Output Structure - -### 📋 Summary -2-3 sentences describing what the PR does and overall assessment. - -### 🔍 Findings -Group by severity. Each finding must include `file:line` reference and concrete explanation. - -**🔴 High** (must fix before merge): -- Security vulnerabilities, data corruption risks, breaking changes without migration - -**🟡 Medium** (should fix before merge): -- Missing error handling, edge cases, test gaps for new behavior - -**🟢 Low** (track for follow-up): -- Minor improvements, non-blocking suggestions with clear rationale - -For each finding: -1. **What**: The specific problem -2. **Why**: How it can be triggered or why it matters -3. **Fix**: Concrete fix approach (describe, don't write implementation code) - -### 🔒 Security Checklist -| Check | Status | Notes | -|-------|--------|-------| -| Injection safety | ✅/❌ | ... | -| Auth checks | ✅/❌/N/A | ... | -| Race conditions | ✅/❌/N/A | ... | -| Secrets exposure | ✅/❌ | ... | -| Supply chain | ✅/❌/N/A | ... | - -### 📊 CCS Compliance -| Rule | Status | Notes | -|------|--------|-------| -| No emojis in CLI | ✅/❌/N/A | ... | -| Test isolation | ✅/❌/N/A | ... | -| Cross-platform | ✅/❌/N/A | ... | -| --help updated | ✅/❌/N/A | ... | -| Settings strings | ✅/❌/N/A | ... | -| Conventional commit | ✅/❌ | ... | -| Docs mandatory | ✅/❌/N/A | ... | - -### 💡 Informational -Non-blocking observations. - -### ✅ What's Done Well -2-3 items max, only if genuinely noteworthy. OPTIONAL — skip if nothing stands out. - -### 🎯 Overall Assessment - -**✅ APPROVED** — zero High, zero security Medium, all CCS rules respected, tests exist for new behavior. -**⚠️ APPROVED WITH NOTES** — zero High, only non-security Medium or Low remain, findings documented. -**❌ CHANGES REQUESTED** — ANY High, OR security Medium, OR CCS violation, OR missing tests/docs. - -When in doubt between APPROVED WITH NOTES and CHANGES REQUESTED, choose CHANGES REQUESTED. +- Return confirmed findings only. +- Every finding must cite a file path and, when practical, a line number. +- Keep the total finding count small unless the PR genuinely has several distinct problems. +- If there are no confirmed findings, say so in the summary and return an empty findings array. +- Use `approved` only when the diff is ready to merge as-is. +- Use `approved_with_notes` when only non-blocking follow-ups remain. +- Use `changes_requested` when any blocking issue remains. diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index cdd917ba..65980150 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -151,6 +151,8 @@ jobs: MAX_THINKING_TOKENS: '16000' REVIEW_OUTPUT_FILE: pr_review.md REVIEW_COMMENT_FILE: .ccs-ai-review-comment.md + REVIEW_OUTPUT_SCHEMA: >- + {"type":"object","additionalProperties":false,"properties":{"summary":{"type":"string","minLength":1},"findings":{"type":"array","maxItems":6,"items":{"type":"object","additionalProperties":false,"properties":{"severity":{"type":"string","enum":["high","medium","low"]},"title":{"type":"string","minLength":1},"file":{"type":"string","minLength":1},"line":{"type":["integer","null"],"minimum":1},"what":{"type":"string","minLength":1},"why":{"type":"string","minLength":1},"fix":{"type":"string","minLength":1}},"required":["severity","title","file","what","why","fix"]}},"overallAssessment":{"type":"string","enum":["approved","approved_with_notes","changes_requested"]},"overallRationale":{"type":"string","minLength":1},"notes":{"type":"array","maxItems":4,"items":{"type":"string","minLength":1}}},"required":["summary","findings","overallAssessment","overallRationale"]} steps: - name: Prepare isolated Claude runtime @@ -224,7 +226,8 @@ jobs: anthropic_api_key: ${{ secrets.GLM_API_KEY }} github_token: ${{ steps.app-token.outputs.token }} allowed_non_write_users: ${{ needs.prepare.outputs.contributor_source == 'external' && '*' || '' }} - show_full_output: true # Visible logs for debugging slow/failing reviews + display_report: false # Keep all public review output on the normalized comment path + show_full_output: false # Keep scratch output out of public logs track_progress: false # Disabled - no progress comments, just final review prompt: | think @@ -242,55 +245,32 @@ jobs: ${{ steps.review-prompt.outputs.content }} - ## IMPORTANT: Writing the Review - After completing your analysis, use the `Write` tool to write the final review markdown to `${{ env.REVIEW_OUTPUT_FILE }}`. - Do NOT use `Edit` tool — use `Write` tool directly to create the file in one shot. - Do NOT post any GitHub comments yourself. The workflow will publish the saved file. - Do NOT modify any source code files — this is a READ-ONLY review. - - End your review with: - > 🤖 Reviewed by `${{ env.REVIEW_MODEL }}` - - IMPORTANT RULES: - - Use `Write` tool to overwrite `${{ env.REVIEW_OUTPUT_FILE }}` with the complete review - - Do NOT use shell operators like || or && in bash commands - - Do NOT use heredoc (<<) syntax in bash commands - - Use simple, single-purpose bash commands only + ## Runtime Rules + - This is a READ-ONLY review. Do not edit files. + - Use the checked-out PR branch plus surrounding repository context before reporting a finding. + - Return only structured output that matches the provided JSON schema. + - Do NOT write files. + - Do NOT post GitHub comments yourself. + - If no confirmed issues remain, return an empty findings array instead of inventing low-value feedback. claude_args: | --bare --model ${{ env.REVIEW_MODEL }} --permission-mode bypassPermissions --max-turns 40 - --allowedTools "Glob,Grep,Read,Write,Bash(gh pr diff *),Bash(gh pr view *),Bash(git diff *),Bash(git log *),Bash(git show *),Bash(cat *),Bash(ls *),Bash(wc *),Bash(head *),Bash(tail *),Bash(find *)" + --allowedTools "Read,Bash(gh pr diff:*),Bash(gh pr view:*),Bash(git diff:*),Bash(git log:*),Bash(git show:*),Bash(cat:*),Bash(ls:*),Bash(wc:*),Bash(head:*),Bash(tail:*),Bash(find:*),Bash(grep:*)" + --json-schema '${{ env.REVIEW_OUTPUT_SCHEMA }}' - # Fallback: if Claude didn't write the review file, extract from execution output - - name: Extract review from execution output (fallback) + - name: Render review comment if: always() && steps.claude-review.outcome != 'cancelled' run: | - EXEC_LOG="$RUNNER_TEMP/claude-execution-output.json" - if [ -s "$REVIEW_OUTPUT_FILE" ]; then - echo "[i] Review file exists, skipping fallback extraction" - exit 0 - fi - if [ ! -f "$EXEC_LOG" ]; then - echo "::warning::No execution output found at $EXEC_LOG" - exit 0 - fi - # Extract last assistant text message as fallback review - EXTRACTED=$(jq -r ' - [.[] | select(.type == "assistant") | .message.content[]? - | select(.type == "text") | .text] | last // empty - ' "$EXEC_LOG" 2>/dev/null || true) - if [ -z "$EXTRACTED" ]; then - echo "::warning::Could not extract review content from execution output" - printf '## AI Review (incomplete)\n\nClaude completed but did not produce a structured review.\nCheck the [execution log artifact](%s) for details.\n\n> Reviewed by `%s` (fallback extraction)\n' \ - "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" \ - "$REVIEW_MODEL" > "$REVIEW_OUTPUT_FILE" - else - printf '%s\n' "$EXTRACTED" > "$REVIEW_OUTPUT_FILE" - fi - echo "[i] Fallback review extracted from execution output" + node scripts/github/normalize-ai-review-output.mjs + env: + AI_REVIEW_EXECUTION_FILE: ${{ runner.temp }}/claude-execution-output.json + AI_REVIEW_MODEL: ${{ env.REVIEW_MODEL }} + AI_REVIEW_OUTPUT_FILE: ${{ env.REVIEW_OUTPUT_FILE }} + AI_REVIEW_RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + AI_REVIEW_STRUCTURED_OUTPUT: ${{ steps.claude-review.outputs.structured_output }} - name: Publish review comment if: always() && steps.claude-review.outcome != 'cancelled' diff --git a/scripts/github/normalize-ai-review-output.mjs b/scripts/github/normalize-ai-review-output.mjs new file mode 100644 index 00000000..a01211a4 --- /dev/null +++ b/scripts/github/normalize-ai-review-output.mjs @@ -0,0 +1,204 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const ASSESSMENTS = { + approved: '✅ APPROVED', + approved_with_notes: '⚠️ APPROVED WITH NOTES', + changes_requested: '❌ CHANGES REQUESTED', +}; + +const SEVERITY_ORDER = ['high', 'medium', 'low']; +const SEVERITY_HEADERS = { + high: '### 🔴 High', + medium: '### 🟡 Medium', + low: '### 🟢 Low', +}; + +function cleanText(value) { + return typeof value === 'string' ? value.trim().replace(/\s+/g, ' ') : ''; +} + +function escapeMarkdownText(value) { + return cleanText(value).replace(/\\/g, '\\\\').replace(/([`*_{}\[\]<>])/g, '\\$1'); +} + +function renderCode(value) { + const text = cleanText(value); + const longestFence = Math.max(...[...text.matchAll(/`+/g)].map((match) => match[0].length), 0); + const fence = '`'.repeat(longestFence + 1); + return `${fence}${text}${fence}`; +} + +function readExecutionMetadata(executionFile) { + if (!executionFile || !fs.existsSync(executionFile)) { + return {}; + } + + try { + const turns = JSON.parse(fs.readFileSync(executionFile, 'utf8')); + const init = turns.find((turn) => turn?.type === 'system' && turn?.subtype === 'init'); + const result = [...turns].reverse().find((turn) => turn?.type === 'result'); + return { + runtimeTools: Array.isArray(init?.tools) ? init.tools : [], + turnsUsed: typeof result?.num_turns === 'number' ? result.num_turns : null, + }; + } catch { + return {}; + } +} + +export function normalizeStructuredOutput(raw) { + if (!raw) { + return { ok: false, reason: 'missing structured output' }; + } + + let parsed; + try { + parsed = typeof raw === 'string' ? JSON.parse(raw) : raw; + } catch { + return { ok: false, reason: 'structured output is not valid JSON' }; + } + + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) { + return { ok: false, reason: 'structured output must be an object' }; + } + + const summary = cleanText(parsed.summary); + const overallAssessment = cleanText(parsed.overallAssessment); + const overallRationale = cleanText(parsed.overallRationale); + const notes = Array.isArray(parsed.notes) ? parsed.notes.map(cleanText).filter(Boolean) : []; + const findings = Array.isArray(parsed.findings) ? parsed.findings : null; + + if (!summary || !ASSESSMENTS[overallAssessment] || !overallRationale || findings === null) { + return { ok: false, reason: 'structured output is missing required review fields' }; + } + + const normalizedFindings = []; + for (const finding of findings) { + const severity = cleanText(finding?.severity); + const title = cleanText(finding?.title); + const file = cleanText(finding?.file); + const what = cleanText(finding?.what); + const why = cleanText(finding?.why); + const fix = cleanText(finding?.fix); + const line = + typeof finding?.line === 'number' && Number.isInteger(finding.line) && finding.line > 0 + ? finding.line + : null; + + if (!SEVERITY_HEADERS[severity] || !title || !file || !what || !why || !fix) { + return { ok: false, reason: 'structured output contains an invalid finding' }; + } + + normalizedFindings.push({ severity, title, file, line, what, why, fix }); + } + + return { + ok: true, + value: { + summary, + findings: normalizedFindings, + overallAssessment, + overallRationale, + notes, + }, + }; +} + +export function renderStructuredReview(review, { model }) { + const lines = ['## Summary', escapeMarkdownText(review.summary), '', '## Findings']; + + if (review.findings.length === 0) { + lines.push('No confirmed issues found after reviewing the diff and surrounding code.'); + } else { + for (const severity of SEVERITY_ORDER) { + const findings = review.findings.filter((finding) => finding.severity === severity); + if (findings.length === 0) continue; + + lines.push(SEVERITY_HEADERS[severity]); + for (const finding of findings) { + const location = finding.line ? `${finding.file}:${finding.line}` : finding.file; + lines.push(`- **${renderCode(location)} — ${escapeMarkdownText(finding.title)}**`); + lines.push(` Problem: ${escapeMarkdownText(finding.what)}`); + lines.push(` Why it matters: ${escapeMarkdownText(finding.why)}`); + lines.push(` Suggested fix: ${escapeMarkdownText(finding.fix)}`); + } + lines.push(''); + } + if (lines[lines.length - 1] === '') { + lines.pop(); + } + } + + if (review.notes.length > 0) { + lines.push('', '## Notes'); + for (const note of review.notes) { + lines.push(`- ${escapeMarkdownText(note)}`); + } + } + + lines.push( + '', + '## Overall Assessment', + `**${ASSESSMENTS[review.overallAssessment]}** — ${escapeMarkdownText(review.overallRationale)}`, + '', + `> Reviewed by \`${model}\`` + ); + + return lines.join('\n'); +} + +export function renderIncompleteReview({ model, reason, runUrl, runtimeTools, turnsUsed }) { + const lines = [ + '## AI Review Incomplete', + '', + 'Claude did not return validated structured review output, so this workflow did not publish raw scratch text.', + '', + `- Reason: ${escapeMarkdownText(reason)}`, + ]; + + if (runtimeTools?.length) { + lines.push(`- Runtime tools: ${runtimeTools.map(renderCode).join(', ')}`); + } + if (typeof turnsUsed === 'number') { + lines.push(`- Turns used: ${turnsUsed}`); + } + + lines.push('', `Re-run \`/review\` or inspect [the workflow run](${runUrl}).`, '', `> Reviewed by \`${model}\``); + return lines.join('\n'); +} + +export function writeReviewFromEnv(env = process.env) { + const outputFile = env.AI_REVIEW_OUTPUT_FILE || 'pr_review.md'; + const model = env.AI_REVIEW_MODEL || 'unknown-model'; + const runUrl = env.AI_REVIEW_RUN_URL || '#'; + const validation = normalizeStructuredOutput(env.AI_REVIEW_STRUCTURED_OUTPUT); + const metadata = readExecutionMetadata(env.AI_REVIEW_EXECUTION_FILE); + const content = validation.ok + ? renderStructuredReview(validation.value, { model }) + : renderIncompleteReview({ + model, + reason: validation.reason, + runUrl, + runtimeTools: metadata.runtimeTools, + turnsUsed: metadata.turnsUsed, + }); + + fs.mkdirSync(path.dirname(outputFile), { recursive: true }); + fs.writeFileSync(outputFile, `${content}\n`, 'utf8'); + + if (!validation.ok) { + console.warn(`::warning::AI review output normalization fell back to incomplete comment: ${validation.reason}`); + } + + return { usedFallback: !validation.ok, content }; +} + +const isMain = + process.argv[1] && + path.resolve(process.argv[1]) === path.resolve(fileURLToPath(import.meta.url)); + +if (isMain) { + writeReviewFromEnv(); +} diff --git a/tests/unit/scripts/github/normalize-ai-review-output.test.ts b/tests/unit/scripts/github/normalize-ai-review-output.test.ts new file mode 100644 index 00000000..b81f3a22 --- /dev/null +++ b/tests/unit/scripts/github/normalize-ai-review-output.test.ts @@ -0,0 +1,117 @@ +import { describe, expect, test } from 'bun:test'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; + +const reviewOutput = await import('../../../../scripts/github/normalize-ai-review-output.mjs'); + +describe('normalize-ai-review-output', () => { + test('renders validated structured output into stable markdown', () => { + const validation = reviewOutput.normalizeStructuredOutput( + JSON.stringify({ + summary: 'The PR is mostly correct, but one blocking regression remains.', + findings: [ + { + severity: 'high', + title: 'Ambiguous account lookup drops valid matches', + file: 'src/cliproxy/accounts/query.ts', + line: 61, + what: 'Exact email matches can return null when duplicate accounts exist.', + why: 'That breaks normal selection flows for users with multiple Codex sessions.', + fix: 'Match by stable account identity first and keep ambiguous email lookups out of exact-match paths.', + }, + ], + overallAssessment: 'changes_requested', + overallRationale: 'The blocking lookup regression should be fixed before merge.', + notes: ['Docs update is present and looks aligned with the code changes.'], + }) + ); + + expect(validation.ok).toBe(true); + const markdown = reviewOutput.renderStructuredReview(validation.value, { model: 'glm-5.1' }); + + expect(markdown).toContain('## Summary'); + expect(markdown).toContain('### 🔴 High'); + expect(markdown).toContain('`src/cliproxy/accounts/query.ts:61`'); + expect(markdown).toContain('**❌ CHANGES REQUESTED**'); + expect(markdown).toContain('Why it matters: That breaks normal selection flows for users with multiple Codex sessions.'); + expect(markdown).toContain('> Reviewed by `glm-5.1`'); + }); + + test('writes a safe incomplete comment instead of leaking raw assistant text', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ai-review-')); + const executionFile = path.join(tempDir, 'claude-execution-output.json'); + const outputFile = path.join(tempDir, 'pr_review.md'); + + fs.writeFileSync( + executionFile, + JSON.stringify([ + { type: 'system', subtype: 'init', tools: ['Bash', 'Edit', 'Read'] }, + { + type: 'result', + subtype: 'success', + num_turns: 25, + result: 'Now let me verify the findings before I finalize the review...', + }, + ]) + ); + + const result = reviewOutput.writeReviewFromEnv({ + AI_REVIEW_EXECUTION_FILE: executionFile, + AI_REVIEW_MODEL: 'glm-5.1', + AI_REVIEW_OUTPUT_FILE: outputFile, + AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/23758377592', + AI_REVIEW_STRUCTURED_OUTPUT: '', + }); + + expect(result.usedFallback).toBe(true); + + const markdown = fs.readFileSync(outputFile, 'utf8'); + expect(markdown).toContain('## AI Review Incomplete'); + expect(markdown).toContain('Runtime tools: `Bash`, `Edit`, `Read`'); + expect(markdown).toContain('Turns used: 25'); + expect(markdown).not.toContain('Now let me verify the findings'); + }); + + test('escapes markdown-looking content and ignores malformed execution metadata', () => { + const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'ai-review-')); + const executionFile = path.join(tempDir, 'claude-execution-output.json'); + const outputFile = path.join(tempDir, 'pr_review.md'); + + fs.writeFileSync(executionFile, '{not valid json'); + + const result = reviewOutput.writeReviewFromEnv({ + AI_REVIEW_EXECUTION_FILE: executionFile, + AI_REVIEW_MODEL: 'glm-5.1', + AI_REVIEW_OUTPUT_FILE: outputFile, + AI_REVIEW_RUN_URL: 'https://github.com/kaitranntt/ccs/actions/runs/1', + AI_REVIEW_STRUCTURED_OUTPUT: JSON.stringify({ + summary: 'Summary with `code` and ## heading markers.', + findings: [ + { + severity: 'low', + title: 'Title with `ticks`', + file: 'src/example.ts', + line: 9, + what: 'Problem text uses **bold** markers.', + why: 'Why text uses [link] syntax.', + fix: 'Fix text uses markers.', + }, + ], + overallAssessment: 'approved_with_notes', + overallRationale: 'Rationale keeps `_formatting_` stable.', + notes: ['Note with `inline code`.'], + }), + }); + + expect(result.usedFallback).toBe(false); + + const markdown = fs.readFileSync(outputFile, 'utf8'); + expect(markdown).toContain('Summary with \\`code\\` and ## heading markers.'); + expect(markdown).toContain('**`src/example.ts:9` — Title with \\`ticks\\`**'); + expect(markdown).toContain('Problem: Problem text uses \\*\\*bold\\*\\* markers.'); + expect(markdown).toContain('Why it matters: Why text uses \\[link\\] syntax.'); + expect(markdown).toContain('Suggested fix: Fix text uses \\ markers.'); + expect(markdown).toContain('**⚠️ APPROVED WITH NOTES**'); + }); +});