fix(#2054): synthesize review body when findings contradict summary

[fullsend-ai-coder]

When the review agent produces a result where the action is request-changes with critical/high findings but the body omits those findings (e.g. says "No findings"), the sticky comment misleads reviewers into thinking the review is clean.

The previous approach (PR #2055, closed) used regex replacement to patch "No findings" text in-place. This was fragile: the regex could match inside longer phrases, ReplaceAllString could duplicate content, and inserting bullet lists mid-sentence produced malformed markdown.

This fix takes a different approach. Instead of string surgery, ensureBodyFindingsConsistency checks whether the body references any critical/high finding categories (case-insensitive substring match on hyphenated tokens like "logic-error", "auth-bypass"). If none are referenced, the entire body is replaced with one synthesized from the structured findings array using the standard review format from the pr-review skill.

The pr-review skill is also updated with an explicit instruction that when action is request-changes or reject, the body MUST list the findings — fixing the issue closer to the source while the CLI provides a safety net.

Note: pre-commit could not run in the sandbox due to shellcheck network restrictions (infrastructure issue, not code issue).

---

Closes #2054

Post-script verification

• Branch is not main/master (agent/2054-review-summary-consistency)
• Secret scan passed (gitleaks — b566a235ef4cfdfdde0c639daa87da585be08e91..HEAD)
• Pre-commit hooks passed (authoritative run on runner)
• Tests ran inside sandbox

[github-actions]

E2E tests did not run

E2E tests run automatically for org/repo members and collaborators on pull requests.

For other contributors, a maintainer must add the ok-to-test label after the latest push.

See E2E testing guide for details.

[github-actions]

Site preview

Preview: https://5639967b-site.fullsend-ai.workers.dev

Commit: dad117730ebe1cdc75ee1f1ec7180fe8911c3735

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 5:13 PM UTC · Completed 5:27 PM UTC
Commit: dad1177 · View workflow run →

[codecov]

Codecov Report

❌ Patch coverage is 96.07843% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/cli/postreview.go	96.07%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[fullsend-ai-review]

Review

Findings

Medium

• [logic-error] internal/cli/postreview.go — ensureBodyFindingsConsistency returns false (no synthesis needed) as soon as ANY single significant finding's category appears in the body. When multiple critical/high findings exist with different categories and the body mentions only one of them, the function short-circuits and the other critical/high findings remain invisible in the summary. This is the same class of bug the PR is trying to fix — a body that mentions one finding but omits others still misleads reviewers.
  Remediation: Change the early-return logic to require that ALL significant findings have their category referenced in the body, not just any one. Replace the loop that returns false on first match with one that counts how many significant findings are referenced, and only return false if all are covered.

• [logic-error] internal/cli/postreview.go — When ensureBodyFindingsConsistency replaces result.Body with the synthesized body, the <!-- **Head SHA:** <sha> --> HTML comment embedded in the original body's first line is lost. The pre-fetch-prior-review.sh script extracts this comment via regex for re-review SHA anchoring. Without it, future re-reviews cannot identify the prior review SHA, breaking severity anchoring. No test covers this scenario.
  Remediation: Before replacing the body, extract the head SHA HTML comment from the original body, or use result.HeadSHA to construct <!-- **Head SHA:** <sha> --> and prepend it to the synthesized body.

Low

• [edge-case] internal/cli/postreview.go — synthesizeReviewBody renders findings with empty Category as - **[]** (bold empty brackets), producing malformed Markdown.

• [error-handling-idiom] internal/cli/postreview.go — The guard if !ok || event != "REQUEST_CHANGES" combines two conditions. Splitting into separate guard clauses would improve readability.

• [api-shape] internal/cli/postreview.go — ensureBodyFindingsConsistency modifies result.Body in-place and returns a bool. Consider returning (newBody string, patched bool) to make the mutation explicit at the call site.

• [naming-convention] internal/cli/postreview.go — Variable significant could be more descriptive (e.g., significantFindings or criticalHighFindings).

• [design-direction] internal/cli/postreview.go — The consistency check runs before the stale-head check. If stale-head is detected, the synthesis work is wasted.

Info

• [pattern-inconsistency] internal/cli/postreview.go — The order slice in synthesizeReviewBody is recreated on every call. Could be a package-level var.

[fullsend-ai-review]

See the review comment for full details.