Enhance pr-finalize skill with code review phase and safety rules

.github/copilot-instructions.md

@@ -238,10 +238,11 @@ Skills are modular capabilities that can be invoked directly or used by agents.
    - **Categories**: P/0, milestoned, partner, community, recent, docs-maui
 
 3. **pr-finalize** (`.github/skills/pr-finalize/SKILL.md`)
-   - **Purpose**: Verifies PR title and description match actual implementation. Works on any PR. Optionally updates agent session markdown if present.
+   - **Purpose**: Verifies PR title and description match actual implementation, AND performs code review for best practices before merge.
    - **Trigger phrases**: "finalize PR #XXXXX", "check PR description for #XXXXX", "review commit message"
    - **Used by**: Before merging any PR, when description may be stale
    - **Note**: Does NOT require agent involvement or session markdown - works on any PR
+   - **🚨 CRITICAL**: NEVER use `--approve` or `--request-changes` - only post comments. Approval is a human decision.
 
 4. **learn-from-pr** (`.github/skills/learn-from-pr/SKILL.md`)
    - **Purpose**: Analyzes completed PR to identify repository improvements (analysis only, no changes applied)

.github/scripts/Review-PR.ps1

@@ -29,27 +29,31 @@
     If specified, skips merging the PR into the current branch (useful if already merged)
 
 .PARAMETER Interactive
-    If specified, starts Copilot in interactive mode with the prompt (default).
-    Use -NoInteractive for non-interactive mode that exits after completion.
-
-.PARAMETER NoInteractive
-    If specified, runs in non-interactive mode (exits after completion).
-    Requires --allow-all for tool permissions.
+    If specified, starts Copilot in interactive mode with the prompt.
+    Default is non-interactive mode (exits after completion).
 
 .PARAMETER DryRun
     If specified, shows what would be done without making changes
 
+.PARAMETER RunFinalize
+    If specified, runs the pr-finalize skill after the PR agent completes
+    to verify PR title/description match the implementation.
+
+.PARAMETER PostSummaryComment
+    If specified, runs the ai-summary-comment skill after all other phases complete
+    to post a combined summary comment on the PR from all phases.
+
 .EXAMPLE
     .\Review-PR.ps1 -PRNumber 33687
-    Reviews PR #33687 interactively using the default platform (android)
+    Reviews PR #33687 in non-interactive mode (default) using auto-detected platform
 
 .EXAMPLE
     .\Review-PR.ps1 -PRNumber 33687 -Platform ios -SkipMerge
-    Reviews PR #33687 on iOS without merging (assumes already merged), in interactive mode
+    Reviews PR #33687 on iOS without merging (assumes already merged)
 
 .EXAMPLE
-    .\Review-PR.ps1 -PRNumber 33687 -NoInteractive
-    Reviews PR #33687 in non-interactive mode (exits after completion)
+    .\Review-PR.ps1 -PRNumber 33687 -Interactive
+    Reviews PR #33687 in interactive mode (stays open for follow-up questions)
 
 .NOTES
     Prerequisites:
@@ -71,10 +75,16 @@ param(
     [switch]$SkipMerge,
 
     [Parameter(Mandatory = $false)]
-    [switch]$NoInteractive,
+    [switch]$Interactive,
+
+    [Parameter(Mandatory = $false)]
+    [switch]$DryRun,
+
+    [Parameter(Mandatory = $false)]
+    [switch]$PostSummaryComment,
 
     [Parameter(Mandatory = $false)]
-    [switch]$DryRun
+    [switch]$RunFinalize
 )
 
 $ErrorActionPreference = 'Stop'
@@ -230,6 +240,13 @@ Review PR #$PRNumber using the pr agent workflow.
 
 $platformInstruction
 
+🚨 **CRITICAL - NEVER MODIFY GIT STATE:**
+- NEVER run ``git checkout``, ``git switch``, ``git fetch``, ``git stash``, or ``git reset``
+- NEVER run ``git push`` - you do NOT have permission to push anything
+- You are ALWAYS on the correct branch already - the script handles this
+- If the state file says "wrong branch", that's stale state - delete it and start fresh
+- If you think you need to switch branches or push changes, you are WRONG - ask the user instead
+
 **Instructions:**
 1. Read the plan template at ``$planTemplatePath`` for the 5-phase workflow
 2. Read ``.github/agents/pr.md`` for Phases 1-3 instructions
@@ -254,7 +271,7 @@ if ($DryRun) {
     Write-Host "[DRY RUN] Would invoke Copilot CLI with:" -ForegroundColor Magenta
     Write-Host ""
     Write-Host "  Agent: pr" -ForegroundColor Gray
-    Write-Host "  Mode: $(if ($NoInteractive) { 'Non-interactive (-p)' } else { 'Interactive (-i)' })" -ForegroundColor Gray
+    Write-Host "  Mode: $(if ($Interactive) { 'Interactive (-i)' } else { 'Non-interactive (-p)' })" -ForegroundColor Gray
     Write-Host "  PR: #$PRNumber" -ForegroundColor Gray
     Write-Host "  Platform: $(if ($Platform) { $Platform } else { '(agent will determine)' })" -ForegroundColor Gray
     Write-Host ""
@@ -264,7 +281,7 @@ if ($DryRun) {
     Write-Host "To run for real, remove the -DryRun flag" -ForegroundColor Yellow
 } else {
     Write-Host "╔═══════════════════════════════════════════════════════════╗" -ForegroundColor Green
-    Write-Host "║  LAUNCHING COPILOT CLI                                    ║" -ForegroundColor Green
+    Write-Host "║  PHASE 1: PR AGENT REVIEW                                 ║" -ForegroundColor Green
     Write-Host "╚═══════════════════════════════════════════════════════════╝" -ForegroundColor Green
     Write-Host ""
     Write-Host "PR Review Context:" -ForegroundColor Cyan
@@ -274,7 +291,16 @@ if ($DryRun) {
     Write-Host "  PLAN_TEMPLATE:  $planTemplatePath" -ForegroundColor White
     Write-Host "  CURRENT_BRANCH: $(git branch --show-current)" -ForegroundColor White
     Write-Host "  PR_TITLE:       $($prInfo.title)" -ForegroundColor White
-    Write-Host "  MODE:           $(if ($NoInteractive) { 'Non-interactive' } else { 'Interactive' })" -ForegroundColor White
+    Write-Host "  MODE:           $(if ($Interactive) { 'Interactive' } else { 'Non-interactive' })" -ForegroundColor White
+    Write-Host ""
+    Write-Host "Workflow:" -ForegroundColor Cyan
+    Write-Host "  1. PR Agent Review (this phase)" -ForegroundColor White
+    if ($RunFinalize) {
+        Write-Host "  2. pr-finalize skill (queued)" -ForegroundColor White
+    }
+    if ($PostSummaryComment) {
+        Write-Host "  3. ai-summary-comment skill (queued)" -ForegroundColor White
+    }
     Write-Host ""
     Write-Host "─────────────────────────────────────────────────────────────" -ForegroundColor DarkGray
     Write-Host ""
@@ -297,14 +323,14 @@ if ($DryRun) {
     # Add logging options
     $copilotArgs += @("--log-dir", $prLogDir, "--log-level", "info")
 
-    if ($NoInteractive) {
-        # Non-interactive mode: -p with --allow-all
+    if ($Interactive) {
+        # Interactive mode: -i to start with prompt
+        $copilotArgs += @("-i", $prompt)
+    } else {
+        # Non-interactive mode (default): -p with --allow-all
         # Also save session to markdown for review
         $sessionFile = Join-Path $prLogDir "session-$(Get-Date -Format 'yyyyMMdd-HHmmss').md"
         $copilotArgs += @("-p", $prompt, "--allow-all", "--share", $sessionFile)
-    } else {
-        # Interactive mode: -i to start with prompt
-        $copilotArgs += @("-i", $prompt)
     }
 
     Write-Host "🚀 Starting Copilot CLI..." -ForegroundColor Yellow
@@ -322,14 +348,72 @@ if ($DryRun) {
     } else {
         Write-Host "⚠️ Copilot CLI exited with code: $exitCode" -ForegroundColor Yellow
     }
+
+    # Post-completion skills (only run if main agent completed successfully)
+    if ($exitCode -eq 0) {
+
+        # Phase 2: Run pr-finalize skill if requested
+        if ($RunFinalize) {
+            Write-Host ""
+            Write-Host "╔═══════════════════════════════════════════════════════════╗" -ForegroundColor Magenta
+            Write-Host "║  PHASE 2: PR-FINALIZE SKILL                               ║" -ForegroundColor Magenta
+            Write-Host "╚═══════════════════════════════════════════════════════════╝" -ForegroundColor Magenta
+            Write-Host ""
+
+            $finalizePrompt = "Run the pr-finalize skill for PR #$PRNumber. Verify the PR title and description match the actual implementation. Do NOT post a comment - just update the state file at CustomAgentLogsTmp/PRState/pr-$PRNumber.md with your findings."
+
+            $finalizeArgs = @(
+                "-p", $finalizePrompt,
+                "--allow-all",
+                "--stream", "on"
+            )
+
+            Write-Host "🔍 Running pr-finalize..." -ForegroundColor Yellow
+            & copilot @finalizeArgs
+
+            $finalizeExit = $LASTEXITCODE
+            if ($finalizeExit -eq 0) {
+                Write-Host "✅ pr-finalize completed" -ForegroundColor Green
+            } else {
+                Write-Host "⚠️ pr-finalize exited with code: $finalizeExit" -ForegroundColor Yellow
+            }
+        }
+
+        # Phase 3: Run ai-summary-comment skill if requested (posts combined results)
+        if ($PostSummaryComment) {
+            Write-Host ""
+            Write-Host "╔═══════════════════════════════════════════════════════════╗" -ForegroundColor Magenta
+            Write-Host "║  PHASE 3: POST SUMMARY COMMENT                            ║" -ForegroundColor Magenta
+            Write-Host "╚═══════════════════════════════════════════════════════════╝" -ForegroundColor Magenta
+            Write-Host ""
+
+            $commentPrompt = "Use the ai-summary-comment skill to post a comment on PR #$PRNumber based on the results from the PR agent review and pr-finalize phases in CustomAgentLogsTmp/PRState/pr-$PRNumber.md."
+
+            $commentArgs = @(
+                "-p", $commentPrompt,
+                "--allow-all",
+                "--stream", "on"
+            )
+
+            Write-Host "💬 Posting summary comment..." -ForegroundColor Yellow
+            & copilot @commentArgs
+
+            $commentExit = $LASTEXITCODE
+            if ($commentExit -eq 0) {
+                Write-Host "✅ Summary comment posted" -ForegroundColor Green
+            } else {
+                Write-Host "⚠️ ai-summary-comment exited with code: $commentExit" -ForegroundColor Yellow
+            }
+        }
+    }
 }
 
 Write-Host ""
 Write-Host "📝 State file: CustomAgentLogsTmp/PRState/pr-$PRNumber.md" -ForegroundColor Gray
 Write-Host "📋 Plan template: $planTemplatePath" -ForegroundColor Gray
 if (-not $DryRun) {
     Write-Host "📁 Copilot logs: CustomAgentLogsTmp/PRState/$PRNumber/copilot-logs/" -ForegroundColor Gray
-    if ($NoInteractive) {
+    if (-not $Interactive) {
         Write-Host "📄 Session markdown: $sessionFile" -ForegroundColor Gray
     }
 }

.github/skills/ai-summary-comment/SKILL.md

@@ -56,11 +56,17 @@ Most scripts post to the **same single comment** identified by `<!-- AI Summary
 
 ### Separate PR Finalization Comment
 
-The `post-pr-finalize-comment.ps1` script posts a **separate comment** identified by `<!-- PR-FINALIZE-COMMENT -->`. This comment contains two sections:
-- **Title**: Shows the suggested PR title with comparison to current
-- **Description**: Shows the suggested PR description
+The `post-pr-finalize-comment.ps1` script posts a **separate comment** identified by `<!-- PR-FINALIZE-COMMENT -->`. This comment contains three sections:
+- **Title**: Shows the current vs recommended PR title
+- **Description**: Shows description assessment, missing elements, and **recommended description**
+- **Code Review**: Shows code review findings (critical issues, suggestions, positive observations)
 
-If an existing finalize comment exists, it will be replaced with the updated Title and Description sections. This keeps finalization reviews distinct from automated analysis.
+If an existing finalize comment exists, it will be replaced with the updated sections. This keeps finalization reviews distinct from automated analysis.
+
+**⚠️ Important Requirements for PR Finalize Comments:**
+- When `TitleStatus` is `NeedsUpdate`, **always provide** `-RecommendedTitle`
+- When `DescriptionStatus` is `NeedsUpdate` or `NeedsRewrite`, **always provide** `-RecommendedDescription` with the full suggested description text
+- The script will warn if these are missing but won't fail
 
 ## Section Scripts
 
@@ -449,6 +455,101 @@ CustomAgentLogsTmp/PRState/{IssueNumber}/write-tests/
 
 ---
 
+## PR Finalize Comment Script
+
+The `post-pr-finalize-comment.ps1` script posts a **separate comment** (not part of the unified AI Summary) specifically for PR finalization reviews. It provides structured feedback on the PR title, description, and code review findings.
+
+### Usage
+
+#### Simplest: Just provide PR number (auto-loads from summary file)
+
+```powershell
+# Auto-loads from CustomAgentLogsTmp/PRState/{PRNumber}/pr-finalize/pr-finalize-summary.md
+pwsh .github/skills/ai-summary-comment/scripts/post-pr-finalize-comment.ps1 -PRNumber 33892
+```
+
+#### Full manual parameters (recommended for best results)
+
+```powershell
+pwsh .github/skills/ai-summary-comment/scripts/post-pr-finalize-comment.ps1 `
+    -PRNumber 33892 `
+    -TitleStatus "NeedsUpdate" `
+    -CurrentTitle "Fix 32650 Image Orientation" `
+    -RecommendedTitle "[iOS][Android] MediaPicker: Fix image orientation when RotateImage=true" `
+    -TitleIssues "- Missing platform tags
+- Doesn't describe the behavior fix" `
+    -DescriptionStatus "NeedsUpdate" `
+    -DescriptionAssessment "The current description is minimal and missing:
+- ❌ Missing NOTE block for testing artifacts
+- ❌ No root cause analysis
+- ❌ No technical details" `
+    -MissingElements "Add the NOTE block, root cause, and technical details." `
+    -RecommendedDescription "> [!NOTE]
+> Are you waiting for this PR? Test it: [Testing PR Builds](link)
+
+### Root Cause
+...description...
+
+### Description of Change
+...details..." `
+    -CodeReviewStatus "IssuesFound" `
+    -CodeReviewFindings "### 🔴 Critical Issues
+**1. Broken indentation**
+- File: \`src/file.cs\`
+- Problem: Inconsistent tabs/spaces
+
+### 🟡 Suggestions
+1. Consider disposing Matrix object
+
+### ✅ Looks Good
+- Proper cleanup in finally block"
+```
+
+### Parameters
+
+| Parameter | Required | Description |
+|-----------|----------|-------------|
+| `PRNumber` | Yes* | Pull request number |
+| `SummaryFile` | No | Path to pr-finalize-summary.md (auto-discovered) |
+| `TitleStatus` | No* | `Good` or `NeedsUpdate` |
+| `CurrentTitle` | No* | Current PR title (fetched from GitHub if not provided) |
+| `RecommendedTitle` | No | **Required if TitleStatus is NeedsUpdate** |
+| `TitleIssues` | No | List of issues with current title |
+| `DescriptionStatus` | No* | `Excellent`, `Good`, `NeedsUpdate`, or `NeedsRewrite` |
+| `DescriptionAssessment` | Yes | Assessment of description quality |
+| `MissingElements` | No | What's missing from the description |
+| `RecommendedDescription` | No | **Required if DescriptionStatus is NeedsUpdate/NeedsRewrite** |
+| `CodeReviewStatus` | No | `Passed`, `IssuesFound`, or `Skipped` |
+| `CodeReviewFindings` | No | Markdown content for code review section |
+| `DryRun` | No | Preview instead of posting |
+
+*At least PRNumber or SummaryFile required. Script auto-detects values when possible.
+
+### ⚠️ Common Mistakes to Avoid
+
+1. **Missing RecommendedTitle when TitleStatus is NeedsUpdate**
+   - The script will warn but still post - always provide a recommended title
+
+2. **Missing RecommendedDescription when DescriptionStatus is NeedsUpdate**
+   - Users need to see what the description SHOULD look like
+
+3. **Code review findings not starting with proper headers**
+   - Always structure with `### 🔴 Critical Issues`, `### 🟡 Suggestions`, `### ✅ Looks Good`
+
+4. **Auto-parsing from summary file getting confused**
+   - When in doubt, provide explicit parameters instead of relying on auto-parsing
+
+### Expected Directory Structure for Auto-Loading
+
+```
+CustomAgentLogsTmp/PRState/{PRNumber}/pr-finalize/
+├── pr-finalize-summary.md      # Main summary (auto-parsed)
+├── recommended-description.md  # Full recommended description (optional)
+└── code-review.md             # Code review findings (optional)
+```
+
+---
+
 ## Technical Details
 
 - Comments identified by HTML marker `<!-- AI Summary -->`