From b528bcbf35bed16bc711ad903c41788e7cf4ad52 Mon Sep 17 00:00:00 2001 From: Tam Nhu Tran Date: Mon, 16 Mar 2026 07:38:39 -0400 Subject: [PATCH] fix(ci): address code review findings for hardened AI review Workflow fixes: - Use randomized heredoc delimiter to prevent output injection - Add file existence guard with fallback inline prompt - Use direct cat instead of echo variable (preserves content exactly) Prompt fixes: - Add 4 missing CCS rules: TTY/NO_COLOR, idempotent installs, dashboard parity, documentation mandatory - Qualify suppression list to exclude security/correctness/CCS concerns - Change severity labels from Critical/Medium/Low to High/Medium/Low to match maintainer feedback loop parser expectations --- .github/review-prompt.md | 14 +++++++++----- .github/workflows/ai-review.yml | 17 +++++++++++++---- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/.github/review-prompt.md b/.github/review-prompt.md index 3eae84ab..7bcb8f24 100644 --- a/.github/review-prompt.md +++ b/.github/review-prompt.md @@ -99,6 +99,10 @@ These are project-specific constraints from CLAUDE.md. Violations are automatic - **Settings format** — all env values in settings MUST be strings (not booleans/objects) to prevent PowerShell crashes - **Conventional commit** — PR title must follow conventional commit format - **Non-invasive** — code must NOT modify `~/.claude/settings.json` without explicit user confirmation +- **TTY-aware colors** — respect `NO_COLOR` env var; detect TTY before using colors +- **Idempotent installs** — all install/setup operations must be safe to run multiple times +- **Dashboard parity** — configuration features MUST have both CLI and Dashboard interfaces +- **Documentation mandatory** — CLI/config changes require `--help` update AND docs update (local `docs/` or CCS docs submodule) ## Informational Checks (Non-Blocking But Report) @@ -124,7 +128,7 @@ These are project-specific constraints from CLAUDE.md. Violations are automatic ## Suppressions — DO NOT Flag These - Style/formatting issues (linter handles this) -- "Consider using X instead of Y" when Y works correctly +- "Consider using X instead of Y" when Y works correctly AND the suggestion has no security, correctness, or CCS-compliance implications - Redundancy that aids readability - Issues already addressed in the diff being reviewed (read the FULL diff first) - "Add a comment explaining why" suggestions — comments rot, code should be self-documenting @@ -141,7 +145,7 @@ Use visual hierarchy with emojis and `---` separators between major sections: ### 🔍 Findings Group by severity. Each finding must include `file:line` reference and concrete explanation. -**🔴 Critical** (must fix before merge): +**🔴 High** (must fix before merge): - Security vulnerabilities, data corruption risks, breaking changes without migration **🟡 Medium** (should fix before merge): @@ -172,18 +176,18 @@ Brief acknowledgment of good patterns (2-3 items max, only if genuinely notewort Use ONE of the following. The criteria are strict: **✅ APPROVED** — ONLY when ALL of these are true: -- Zero 🔴 Critical findings +- Zero 🔴 High findings - Zero 🟡 Medium findings with security implications - All CCS-specific constraints respected - Tests exist for new behavior (if applicable) **⚠️ APPROVED WITH NOTES** — when: -- Zero 🔴 Critical findings +- Zero 🔴 High findings - Only non-security 🟡 Medium or 🟢 Low findings remain - Findings are documented (not ignored) **❌ CHANGES REQUESTED** — when ANY of these: -- Any 🔴 Critical finding exists +- Any 🔴 High finding exists (security, data corruption, breaking changes) - Any security-relevant 🟡 Medium finding exists - Missing tests for new behavior that changes user-facing functionality - Breaking change without documentation diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index 91769ae2..84224f2a 100644 --- a/.github/workflows/ai-review.yml +++ b/.github/workflows/ai-review.yml @@ -185,11 +185,20 @@ jobs: - name: Load review prompt id: review-prompt run: | - PROMPT_CONTENT=$(cat .github/review-prompt.md) + PROMPT_FILE=".github/review-prompt.md" + if [ ! -f "$PROMPT_FILE" ]; then + echo "::warning::review-prompt.md not found — using fallback prompt" + PROMPT_FILE="" + fi + DELIMITER="REVIEW_PROMPT_$(openssl rand -hex 16)" { - echo "content<> "$GITHUB_OUTPUT" - name: Run Claude Code Review