tiennm99/ccs
1
0
Fork 0
mirror of https://github.com/tiennm99/ccs.git synced 2026-08-05 22:24:04 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ci): address code review findings for hardened AI review

Browse Source 
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
This commit is contained in:
Tam Nhu Tran
2026-03-16 07:38:39 -04:00
parent 19f70914a9
commit b528bcbf35
2 changed files with 22 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.github/review-prompt.md
+9 -5
View File
@@ -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 - **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 - **Conventional commit** — PR title must follow conventional commit format
- **Non-invasive** — code must NOT modify `~/.claude/settings.json` without explicit user confirmation - **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) ## 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 ## Suppressions — DO NOT Flag These
- Style/formatting issues (linter handles this) - 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 - Redundancy that aids readability
- Issues already addressed in the diff being reviewed (read the FULL diff first) - 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 - "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 ### 🔍 Findings
Group by severity. Each finding must include `file:line` reference and concrete explanation. 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 - Security vulnerabilities, data corruption risks, breaking changes without migration
**🟡 Medium** (should fix before merge): **🟡 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: Use ONE of the following. The criteria are strict:
**✅ APPROVED** — ONLY when ALL of these are true: **✅ APPROVED** — ONLY when ALL of these are true:
- Zero 🔴 Critical findings - Zero 🔴 High findings
- Zero 🟡 Medium findings with security implications - Zero 🟡 Medium findings with security implications
- All CCS-specific constraints respected - All CCS-specific constraints respected
- Tests exist for new behavior (if applicable) - Tests exist for new behavior (if applicable)
**⚠️ APPROVED WITH NOTES** — when: **⚠️ APPROVED WITH NOTES** — when:
- Zero 🔴 Critical findings - Zero 🔴 High findings
- Only non-security 🟡 Medium or 🟢 Low findings remain - Only non-security 🟡 Medium or 🟢 Low findings remain
- Findings are documented (not ignored) - Findings are documented (not ignored)
**❌ CHANGES REQUESTED** — when ANY of these: **❌ 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 - Any security-relevant 🟡 Medium finding exists
- Missing tests for new behavior that changes user-facing functionality - Missing tests for new behavior that changes user-facing functionality
- Breaking change without documentation - Breaking change without documentation
.github/workflows/ai-review.yml
+13 -4
View File
@@ -185,11 +185,20 @@ jobs:
- name: Load review prompt - name: Load review prompt
id: review-prompt id: review-prompt
run: | 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<<REVIEW_PROMPT_EOF" echo "content<<${DELIMITER}"
echo "$PROMPT_CONTENT" if [ -n "$PROMPT_FILE" ]; then
echo "REVIEW_PROMPT_EOF" cat "$PROMPT_FILE"
else
echo "Perform a thorough code review. Flag security issues, logic errors, and missing error handling. Follow the repository CLAUDE.md for project-specific guidelines."
fi
echo "${DELIMITER}"
} >> "$GITHUB_OUTPUT" } >> "$GITHUB_OUTPUT"
- name: Run Claude Code Review - name: Run Claude Code Review