Track actionable review follow-ups

internal/cli/postreview.go

@@ -2,6 +2,8 @@ package cli
 
 import (
 	"context"
+	"crypto/sha256"
+	"encoding/hex"
 	"encoding/json"
 	"fmt"
 	"os"
@@ -17,6 +19,8 @@ import (
 )
 
 const reviewMarker = "<!-- fullsend:review-agent -->"
+const reviewFollowupSummaryMarker = "<!-- fullsend:review-follow-ups -->"
+const reviewFollowupIssueMarkerPrefix = "<!-- fullsend:review-follow-up:"
 
 var hexSHARe = regexp.MustCompile(`^[0-9a-fA-F]{40}$|^[0-9a-fA-F]{64}$`)
 var reasonRe = regexp.MustCompile(`^[a-zA-Z0-9_-]*$`)
@@ -119,7 +123,11 @@ has moved, a stale-head failure is posted instead.`,
 				return err
 			}
 
-			return submitFormalReview(cmd.Context(), client, owner, repoName, pr, parsed.Action, parsed.HeadSHA, commentURL, dryRun, printer)
+			if err := submitFormalReview(cmd.Context(), client, owner, repoName, pr, parsed.Action, parsed.HeadSHA, commentURL, dryRun, printer); err != nil {
+				return err
+			}
+
+			return postApprovedFollowUpIssues(cmd.Context(), client, owner, repoName, pr, parsed, dryRun, printer)
 		},
 	}
 
@@ -137,10 +145,22 @@ has moved, a stale-head failure is posted instead.`,
 
 // ReviewResult represents a parsed review result file.
 type ReviewResult struct {
-	Body    string `json:"body"`
-	Action  string `json:"action"`   // "approve", "request-changes", "comment", "failure"
-	HeadSHA string `json:"head_sha"` // commit SHA the agent reviewed
-	Reason  string `json:"reason"`   // failure reason (when action is "failure")
+	Body     string          `json:"body"`
+	Action   string          `json:"action"`   // "approve", "request-changes", "comment", "failure"
+	HeadSHA  string          `json:"head_sha"` // commit SHA the agent reviewed
+	Reason   string          `json:"reason"`   // failure reason (when action is "failure")
+	Findings []ReviewFinding `json:"findings"`
+}
+
+// ReviewFinding is the structured form emitted by the review agent.
+type ReviewFinding struct {
+	Severity    string `json:"severity"`
+	Category    string `json:"category"`
+	File        string `json:"file"`
+	Line        int    `json:"line,omitempty"`
+	Description string `json:"description"`
+	Remediation string `json:"remediation,omitempty"`
+	Actionable  bool   `json:"actionable,omitempty"`
 }
 
 // reviewActionToEvent maps a ReviewResult action to a GitHub PR review event.
@@ -284,6 +304,204 @@ func submitFormalReview(ctx context.Context, client forge.Client, owner, repo st
 	return nil
 }
 
+type reviewFollowupIssue struct {
+	finding ReviewFinding
+	issue   *forge.Issue
+	created bool
+}
+
+// postApprovedFollowUpIssues creates GitHub issues for actionable low/info
+// findings after an approval. Blocking and comment-only findings stay in the
+// review itself; this path only preserves non-blocking work that would
+// otherwise disappear after merge.
+func postApprovedFollowUpIssues(ctx context.Context, client forge.Client, owner, repo string, pr int, parsed ReviewResult, dryRun bool, printer *ui.Printer) error {
+	if strings.ToLower(parsed.Action) != "approve" {
+		return nil
+	}
+
+	actionable := actionableApprovedFindings(parsed.Findings)
+	if len(actionable) == 0 {
+		return nil
+	}
+
+	if dryRun {
+		printer.StepInfo(fmt.Sprintf("Dry run — would create %d review follow-up issue(s)", len(actionable)))
+		return nil
+	}
+
+	markers := make(map[string]ReviewFinding, len(actionable))
+	for _, finding := range actionable {
+		markers[reviewFollowupIssueMarker(owner, repo, finding)] = finding
+	}
+
+	printer.StepStart("Checking for existing review follow-up issues")
+	openIssues, err := client.ListOpenIssues(ctx, owner, repo, "type/chore")
+	if err != nil {
+		return fmt.Errorf("listing open issues for review follow-up duplicate detection: %w", err)
+	}
+	existingByMarker := map[string]forge.Issue{}
+	for _, issue := range openIssues {
+		for marker := range markers {
+			if strings.Contains(issue.Body, marker) {
+				if existing, ok := existingByMarker[marker]; ok {
+					printer.StepWarn(fmt.Sprintf("Duplicate review follow-up marker found in issues #%d and #%d; reusing #%d", existing.Number, issue.Number, existing.Number))
+					continue
+				}
+				existingByMarker[marker] = issue
+			}
+		}
+	}
+	printer.StepDone("Duplicate check complete")
+
+	results := make([]reviewFollowupIssue, 0, len(actionable))
+	for _, finding := range actionable {
+		marker := reviewFollowupIssueMarker(owner, repo, finding)
+		if issue, ok := existingByMarker[marker]; ok {
+			issueCopy := issue
+			results = append(results, reviewFollowupIssue{
+				finding: finding,
+				issue:   &issueCopy,
+				created: false,
+			})
+			continue
+		}
+
+		title := reviewFollowupIssueTitle(pr, finding)
+		body := reviewFollowupIssueBody(owner, repo, pr, finding, marker)
+		printer.StepStart("Creating review follow-up issue")
+		issue, err := client.CreateIssue(ctx, owner, repo, title, body, "type/chore")
+		if err != nil {
+			return fmt.Errorf("creating review follow-up issue for %s: %w", reviewFindingLocation(finding), err)
+		}
+		printer.StepDone(fmt.Sprintf("Created follow-up issue #%d", issue.Number))
+		results = append(results, reviewFollowupIssue{
+			finding: finding,
+			issue:   issue,
+			created: true,
+		})
+	}
+
+	return postReviewFollowupSummary(ctx, client, owner, repo, pr, results, printer)
+}
+
+func actionableApprovedFindings(findings []ReviewFinding) []ReviewFinding {
+	actionable := make([]ReviewFinding, 0, len(findings))
+	for _, finding := range findings {
+		severity := strings.ToLower(finding.Severity)
+		if finding.Actionable && (severity == "low" || severity == "info") {
+			actionable = append(actionable, finding)
+		}
+	}
+	return actionable
+}
+
+func postReviewFollowupSummary(ctx context.Context, client forge.Client, owner, repo string, pr int, results []reviewFollowupIssue, printer *ui.Printer) error {
+	if len(results) == 0 {
+		return nil
+	}
+
+	var created []reviewFollowupIssue
+	var existing []reviewFollowupIssue
+	for _, result := range results {
+		if result.created {
+			created = append(created, result)
+		} else {
+			existing = append(existing, result)
+		}
+	}
+
+	var b strings.Builder
+	b.WriteString("## Review follow-ups\n\n")
+	if len(created) > 0 {
+		b.WriteString("Created follow-up issues for actionable non-blocking review findings:\n\n")
+		for _, result := range created {
+			fmt.Fprintf(&b, "- [#%d](%s) — %s\n", result.issue.Number, result.issue.URL, reviewFindingSummary(result.finding))
+		}
+	}
+	if len(existing) > 0 {
+		if len(created) > 0 {
+			b.WriteString("\n")
+		}
+		b.WriteString("Existing follow-up issues already track these findings:\n\n")
+		for _, result := range existing {
+			fmt.Fprintf(&b, "- [#%d](%s) — %s\n", result.issue.Number, result.issue.URL, reviewFindingSummary(result.finding))
+		}
+	}
+
+	cfg := sticky.Config{Marker: reviewFollowupSummaryMarker}
+	if _, err := sticky.Post(ctx, client, owner, repo, pr, b.String(), cfg, printer); err != nil {
+		return fmt.Errorf("posting review follow-up summary: %w", err)
+	}
+	return nil
+}
+
+func reviewFollowupIssueTitle(pr int, finding ReviewFinding) string {
+	summary := reviewFindingSummary(finding)
+	if summary == "" {
+		summary = "actionable review finding"
+	}
+	return fmt.Sprintf("Follow-up from PR #%d: %s", pr, truncate(summary, 88))
+}
+
+func reviewFollowupIssueBody(owner, repo string, pr int, finding ReviewFinding, marker string) string {
+	var b strings.Builder
+	b.WriteString(marker)
+	b.WriteString("\n\n")
+	b.WriteString("## Review follow-up\n\n")
+	fmt.Fprintf(&b, "- PR: https://github.com/%s/%s/pull/%d\n", owner, repo, pr)
+	fmt.Fprintf(&b, "- Severity: `%s`\n", finding.Severity)
+	fmt.Fprintf(&b, "- Category: `%s`\n", finding.Category)
+	fmt.Fprintf(&b, "- Location: `%s`\n", reviewFindingLocation(finding))
+	b.WriteString("\n## Finding\n\n")
+	b.WriteString(strings.TrimSpace(finding.Description))
+	if finding.Remediation != "" {
+		b.WriteString("\n\n## Suggested remediation\n\n")
+		b.WriteString(strings.TrimSpace(finding.Remediation))
+	}
+	b.WriteString("\n\n---\n")
+	b.WriteString("_Generated by the fullsend review agent from an approved PR. The PR was approved because this finding was non-blocking, but it was marked actionable so it is tracked separately._\n")
+	return b.String()
+}
+
+func reviewFollowupIssueMarker(owner, repo string, finding ReviewFinding) string {
+	hash := sha256.New()
+	fmt.Fprintf(hash, "%s/%s\n%s\n%s\n%s\n%d\n%s\n", owner, repo, strings.ToLower(finding.Severity), finding.Category, finding.File, finding.Line, compactWhitespace(finding.Description))
+	return reviewFollowupIssueMarkerPrefix + hex.EncodeToString(hash.Sum(nil)) + " -->"
+}
+
+func reviewFindingLocation(finding ReviewFinding) string {
+	if finding.Line > 0 {
+		return fmt.Sprintf("%s:%d", finding.File, finding.Line)
+	}
+	return finding.File
+}
+
+func reviewFindingSummary(finding ReviewFinding) string {
+	summary := compactWhitespace(finding.Description)
+	if summary != "" {
+		return summary
+	}
+	return compactWhitespace(finding.Category)
+}
+
+func compactWhitespace(s string) string {
+	return strings.Join(strings.Fields(s), " ")
+}
+
+func truncate(s string, max int) string {
+	runes := []rune(s)
+	if len(runes) <= max {
+		return s
+	}
+	if max <= 0 {
+		return ""
+	}
+	if max <= 3 {
+		return string(runes[:max])
+	}
+	return strings.TrimSpace(string(runes[:max-3])) + "..."
+}
+
 // dismissStaleRequestChanges dismisses the most recent CHANGES_REQUESTED
 // review by the authenticated user when the new verdict is softer.
 func dismissStaleRequestChanges(ctx context.Context, client forge.Client, owner, repo string, pr int, newEvent, user string, reviews []forge.PullRequestReview, printer *ui.Printer) {