fix(orchestrator): detect '# Plan' heading to prevent duplicate in PR body

internal/github/fake.go

@@ -37,10 +37,12 @@ type InMemoryFake struct {
 	permissions map[string]string
 	prs         []fakePR
 	reactions   map[int64][]string
+	prReviews   map[int][]PRReview
 
-	// next monotonic IDs for created comments / PRs.
-	nextCommentID int64
-	nextPRNumber  int
+	// next monotonic IDs for created comments / PRs / reviews.
+	nextCommentID  int64
+	nextPRNumber   int
+	nextPRReviewID int64
 }
 
 // NewInMemoryFake constructs an empty InMemoryFake bound to the given
@@ -52,14 +54,16 @@ func NewInMemoryFake(fullName string) *InMemoryFake {
 		panic(err)
 	}
 	return &InMemoryFake{
-		owner:         owner,
-		repo:          repo,
-		issues:        make(map[int]*fakeIssue),
-		comments:      make(map[int][]types.IssueComment),
-		permissions:   make(map[string]string),
-		reactions:     make(map[int64][]string),
-		nextCommentID: 1,
-		nextPRNumber:  1000,
+		owner:          owner,
+		repo:           repo,
+		issues:         make(map[int]*fakeIssue),
+		comments:       make(map[int][]types.IssueComment),
+		permissions:    make(map[string]string),
+		reactions:      make(map[int64][]string),
+		prReviews:      make(map[int][]PRReview),
+		nextCommentID:  1,
+		nextPRNumber:   1000,
+		nextPRReviewID: 1,
 	}
 }
 
@@ -284,6 +288,36 @@ func (f *InMemoryFake) AddReaction(_ context.Context, commentID int64, reaction
 	return nil
 }
 
+// SeedPRReview appends a review to a PR. If r.ID is zero, a new ID is
+// assigned. If r.SubmittedAt is zero, time.Now() is used.
+func (f *InMemoryFake) SeedPRReview(prNumber int, r PRReview) PRReview {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	if r.ID == 0 {
+		r.ID = f.nextPRReviewID
+		f.nextPRReviewID++
+	} else if r.ID >= f.nextPRReviewID {
+		f.nextPRReviewID = r.ID + 1
+	}
+	if r.SubmittedAt.IsZero() {
+		r.SubmittedAt = time.Now().UTC()
+	}
+	f.prReviews[prNumber] = append(f.prReviews[prNumber], r)
+	return r
+}
+
+// ListPRReviews implements Client. Returned reviews are sorted by
+// SubmittedAt ascending (oldest first).
+func (f *InMemoryFake) ListPRReviews(_ context.Context, prNumber int) ([]PRReview, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	rs := f.prReviews[prNumber]
+	out := make([]PRReview, len(rs))
+	copy(out, rs)
+	sort.Slice(out, func(i, j int) bool { return out[i].SubmittedAt.Before(out[j].SubmittedAt) })
+	return out, nil
+}
+
 // containsLabel does a case-insensitive lookup in a label slice. The
 // slice is expected to already be lowercase.
 func containsLabel(labels []string, want string) bool {

internal/github/github.go

@@ -33,6 +33,17 @@ type PullRequest struct {
 	State  string
 }
 
+// PRReview is a normalized PR review returned by Client.ListPRReviews.
+// State is one of "APPROVED", "CHANGES_REQUESTED", "COMMENTED", "DISMISSED",
+// or "PENDING" (mirroring GitHub's review states).
+type PRReview struct {
+	ID          int64
+	User        string
+	State       string
+	Body        string
+	SubmittedAt time.Time
+}
+
 // Client is the minimal GitHub surface used by symphony-go.
 type Client interface {
 	// ListReadyIssues returns open issues carrying readyLabel. Pull
@@ -61,6 +72,8 @@ type Client interface {
 	// FindPRsByHead lists open PRs whose head branch matches the given
 	// branch name (in the repo's owner namespace).
 	FindPRsByHead(ctx context.Context, headBranch string) ([]PullRequest, error)
+	// ListPRReviews returns every review submitted on a PR, oldest first.
+	ListPRReviews(ctx context.Context, prNumber int) ([]PRReview, error)
 	// AddReaction adds a reaction (e.g. "+1", "-1", "eyes") to an issue
 	// comment.
 	AddReaction(ctx context.Context, commentID int64, reaction string) error
@@ -311,6 +324,28 @@ func (r *realClient) FindPRsByHead(ctx context.Context, headBranch string) ([]Pu
 	return out, nil
 }
 
+func (r *realClient) ListPRReviews(ctx context.Context, prNumber int) ([]PRReview, error) {
+	opts := &gh.ListOptions{PerPage: 100}
+	var out []PRReview
+	for {
+		reviews, resp, err := r.c.PullRequests.ListReviews(ctx, r.owner, r.repo, prNumber, opts)
+		if err != nil {
+			return nil, fmt.Errorf("github: list PR reviews on #%d: %w", prNumber, err)
+		}
+		for _, rv := range reviews {
+			if rv == nil {
+				continue
+			}
+			out = append(out, normalizePRReview(rv))
+		}
+		if resp == nil || resp.NextPage == 0 {
+			break
+		}
+		opts.Page = resp.NextPage
+	}
+	return out, nil
+}
+
 func (r *realClient) AddReaction(ctx context.Context, commentID int64, reaction string) error {
 	if _, _, err := r.c.Reactions.CreateIssueCommentReaction(ctx, r.owner, r.repo, commentID, reaction); err != nil {
 		return fmt.Errorf("github: add reaction %q to comment %d: %w", reaction, commentID, err)
@@ -367,3 +402,19 @@ func normalizePR(p *gh.PullRequest) PullRequest {
 		State:  p.GetState(),
 	}
 }
+
+// normalizePRReview converts a go-github *PullRequestReview into PRReview.
+func normalizePRReview(rv *gh.PullRequestReview) PRReview {
+	out := PRReview{
+		ID:    rv.GetID(),
+		Body:  rv.GetBody(),
+		State: rv.GetState(),
+	}
+	if rv.User != nil && rv.User.Login != nil {
+		out.User = *rv.User.Login
+	}
+	if rv.SubmittedAt != nil {
+		out.SubmittedAt = rv.SubmittedAt.Time
+	}
+	return out
+}

internal/orchestrator/job.go

@@ -377,6 +377,9 @@ func (o *Orchestrator) routeAuto(ctx context.Context, job *types.Job, issue type
 	log.Info("reviewer_completed", "decision", dec.Decision)
 
 	if dec.Decision == "approve" {
+		reasons := strings.Join(dec.Reasons, "; ")
+		approveBody := fmt.Sprintf("[symphony-go] ✅ reviewer approved the plan: %s", reasons)
+		_, _ = o.deps.GitHub.PostIssueComment(ctx, issue.Number, approveBody)
 		job.ApprovalPath = types.ApprovalPathReviewer
 		_ = o.saveJob(job)
 		layout := workspace.LayoutFor(o.deps.WorkspaceRoot, issue.Number, workspace.SanitizeSlug(issue.Title))
@@ -594,7 +597,14 @@ func (o *Orchestrator) runImplementation(ctx context.Context, job *types.Job, is
 		return err
 	}
 	job.Status = types.StatusPRReady
-	return o.saveJob(job)
+	if err := o.saveJob(job); err != nil {
+		return err
+	}
+
+	// 18. Supplementary post-PR code review (best-effort, purely
+	// informational — never mutates job state, even on "reject").
+	o.runPostPRCodeReview(ctx, issue, pr.Number)
+	return nil
 }
 
 // runImplementationAgent drives the implementation phase. When multi-turn
@@ -970,8 +980,14 @@ func buildPRBody(job *types.Job, results []valResult, workflowEdited bool, proof
 	fmt.Fprintf(&b, "Resolves #%d.\n\n", job.IssueNumber)
 	fmt.Fprintf(&b, "Approval path: `%s`\n\n", job.ApprovalPath)
 	if job.PlanText != "" {
-		b.WriteString("## Plan\n\n")
-		b.WriteString(job.PlanText)
+		planText := strings.TrimSpace(job.PlanText)
+		// The agent may already include a plan heading at any markdown
+		// level (# Plan, ## Plan, ### Plan). Only add our own if the
+		// text doesn't already start with one.
+		if !hasPlanHeading(planText) {
+			b.WriteString("## Plan\n\n")
+		}
+		b.WriteString(planText)
 		b.WriteString("\n\n")
 	}
 	b.WriteString("## Validation\n\n")
@@ -1211,4 +1227,135 @@ func (o *Orchestrator) resolveValidationCommands(job *types.Job) ([]string, erro
 	return nil, fmt.Errorf("validation.commands_by_label has no entry for %q and no default", axis)
 }
 
+// hasPlanHeading reports whether text starts with a markdown heading
+// containing "Plan" at any level (# Plan, ## Plan, ### Plan, etc.).
+// This prevents buildPRBody from prepending a duplicate heading when the
+// agent already emits one.
+func hasPlanHeading(text string) bool {
+	firstLine := text
+	if i := strings.IndexByte(text, '\n'); i >= 0 {
+		firstLine = text[:i]
+	}
+	firstLine = strings.TrimSpace(firstLine)
+	// Must start with at least one '#'.
+	if !strings.HasPrefix(firstLine, "#") {
+		return false
+	}
+	// Strip leading '#' characters and require a space after them.
+	stripped := strings.TrimLeft(firstLine, "#")
+	if len(stripped) == 0 || stripped[0] != ' ' {
+		return false
+	}
+	// Check that the heading word is "Plan" — either exactly or followed
+	// by a space (e.g. "## Plan: details"). Rejects "Planning", "Planet".
+	lower := strings.ToLower(strings.TrimSpace(stripped))
+	return lower == "plan" || strings.HasPrefix(lower, "plan ")
+}
+
 var _ = errors.New // silence unused import if ever
+
+// runPostPRCodeReview runs the reviewer agent on the actual code diff
+// (not just the plan) and posts the review as a PR comment. Purely
+// informational: it never mutates job state and never triggers a
+// revision. A "reject" decision is surfaced as a "Changes Requested"
+// comment for human follow-up; the orchestration state machine is
+// unaffected. The `job` is intentionally not passed in so the function
+// physically cannot mutate it.
+func (o *Orchestrator) runPostPRCodeReview(ctx context.Context, issue types.Issue, prNumber int) {
+	log := o.deps.Logger.With("issue", issue.Number, "pr", prNumber)
+	cfg := o.deps.Config
+
+	if o.deps.Reviewer == nil {
+		log.Debug("code_review_skipped", "reason", "no reviewer configured")
+		return
+	}
+
+	layout := workspace.LayoutFor(o.deps.WorkspaceRoot, issue.Number, workspace.SanitizeSlug(issue.Title))
+
+	// Fetch origin/<base> so the ref is current. Without this, when
+	// multiple jobs run sequentially and earlier PRs get merged between
+	// runs, origin/<base> can be stale, causing the diff to include
+	// commits from previously-merged PRs.
+	fetchCmd := exec.CommandContext(ctx, "git", "-C", layout.RepoPath, "fetch", "origin", cfg.Repo.BaseBranch)
+	if fetchErr := fetchCmd.Run(); fetchErr != nil {
+		log.Warn("code_review_fetch_failed", "err", fetchErr)
+		// Continue anyway — the diff may still be correct if origin/<base> is recent.
+	}
+
+	// Get the diff against base branch (three-dot, origin-prefixed — matches
+	// gitDiffNameOnly and the rest of the codebase).
+	diffCmd := exec.CommandContext(ctx, "git", "-C", layout.RepoPath, "diff", "origin/"+cfg.Repo.BaseBranch+"...HEAD")
+	diffOut, err := diffCmd.Output()
+	if err != nil {
+		log.Warn("code_review_diff_failed", "err", err)
+		return
+	}
+
+	if len(diffOut) == 0 {
+		log.Debug("code_review_skipped", "reason", "empty diff")
+		return
+	}
+
+	// Truncate large diffs to stay within model context limits.
+	diffStr := string(diffOut)
+	if len(diffStr) > 30000 {
+		diffStr = diffStr[:30000] + "\n... (truncated)"
+	}
+
+	// Build a code review prompt (fixed, not user-controlled). The trailing
+	// `## Decision` + fenced JSON block matches approval.ParseDecision so
+	// the result can be parsed via the standard reviewer flow.
+	prompt := fmt.Sprintf(`You are a code review agent. Review this diff for a GitHub pull request.
+
+Issue: #%d %s
+Description: %s
+
+Diff:
+%s
+
+Review the code for:
+1. Bugs or logic errors
+2. Missing error handling
+3. Style inconsistencies with surrounding code
+4. Test coverage gaps
+5. Security concerns
+
+If the code looks good, say so briefly. If there are issues, list them clearly.
+
+End your response with:
+
+## Decision
+`+"```json\n"+`{"decision": "approve", "reasons": ["summary"]}
+`+"```\n"+`
+Set decision to "approve" if code is acceptable, "reject" if changes needed.
+`, issue.Number, issue.Title, issue.Description, diffStr)
+
+	reviewerHome := filepath.Join(o.deps.WorkspaceRoot, ".symphony-go", "review-homes",
+		fmt.Sprintf("issue-%d-code-review", issue.Number))
+	_ = os.MkdirAll(reviewerHome, 0o755)
+
+	log.Info("code_review_started")
+	result, rerr := o.deps.Reviewer.Review(ctx, approval.ReviewInput{
+		Issue:    issue,
+		PlanText: prompt,
+		RepoPath: layout.RepoPath,
+		HomePath: reviewerHome,
+	})
+
+	if rerr != nil {
+		log.Warn("code_review_failed", "err", rerr)
+		return
+	}
+
+	// Post review as a comment on the PR. Even a "reject" decision is
+	// purely informational here — no triggerRevision, no state changes.
+	reasons := strings.Join(result.Reasons, "\n")
+	var body string
+	if result.Decision == "approve" {
+		body = fmt.Sprintf("[symphony-go] 🔍 **Code Review:** ✅ LGTM\n\n%s", reasons)
+	} else {
+		body = fmt.Sprintf("[symphony-go] 🔍 **Code Review:** ⚠️ Changes Requested\n\n%s", reasons)
+	}
+	_, _ = o.deps.GitHub.PostIssueComment(ctx, prNumber, body)
+	log.Info("code_review_completed", "decision", result.Decision)
+}

internal/orchestrator/loop.go

@@ -34,6 +34,9 @@ func (o *Orchestrator) Run(ctx context.Context, once bool) error {
 		if err := o.PollApprovals(ctx); err != nil {
 			o.deps.Logger.Error("tick: PollApprovals", "err", err)
 		}
+		if err := o.PollPRRevisions(ctx); err != nil {
+			o.deps.Logger.Error("tick: PollPRRevisions", "err", err)
+		}
 		max := o.deps.Config.Orchestrator.MaxConcurrentJobs
 		if max < 1 {
 			max = 1

internal/orchestrator/orchestrator_test.go

@@ -555,6 +555,47 @@ func TestBuildPRBodyIncludesProofArtifacts(t *testing.T) {
 	}
 }
 
+func TestBuildPRBodyNoDuplicatePlanHeading(t *testing.T) {
+	// When the agent includes "## Plan" in its plan text, buildPRBody should
+	// not add a second "## Plan" heading.
+	job := &types.Job{
+		IssueNumber:  42,
+		ApprovalPath: types.ApprovalPathReviewer,
+		PlanText:     "## Plan\n\n### Steps\n- do thing one\n- do thing two",
+	}
+	body := buildPRBody(job, nil, false, nil)
+	count := strings.Count(body, "## Plan")
+	if count != 1 {
+		t.Fatalf("expected exactly 1 '## Plan' heading, got %d:\n%s", count, body)
+	}
+
+	// When the agent does NOT include a heading, buildPRBody should add one.
+	job2 := &types.Job{
+		IssueNumber:  43,
+		ApprovalPath: types.ApprovalPathReviewer,
+		PlanText:     "### Steps\n- do thing one\n- do thing two",
+	}
+	body2 := buildPRBody(job2, nil, false, nil)
+	if !strings.Contains(body2, "## Plan\n\n### Steps") {
+		t.Fatalf("expected '## Plan' heading to be added:\n%s", body2)
+	}
+
+	// When the agent uses "# Plan" (H1), buildPRBody should NOT add a
+	// duplicate heading.
+	job3 := &types.Job{
+		IssueNumber:  44,
+		ApprovalPath: types.ApprovalPathReviewer,
+		PlanText:     "# Plan\n\n## Issue Context\n- details",
+	}
+	body3 := buildPRBody(job3, nil, false, nil)
+	if strings.Contains(body3, "## Plan\n\n# Plan") {
+		t.Fatalf("duplicate heading when agent uses '# Plan':\n%s", body3)
+	}
+	if !strings.Contains(body3, "# Plan\n") {
+		t.Fatalf("expected agent's '# Plan' heading preserved:\n%s", body3)
+	}
+}
+
 func TestEmptyPlanDoesNotPostBlankComment(t *testing.T) {
 	h := newTestHarness(t)
 	h.cfg.Approval.Mode = "handoff"