Agent Skills: PR Finalize

PR Finalize

Ensures PR title and description accurately reflect the implementation, and performs a code review for best practices before merge.

Standalone skill - Can be used on any PR, not just PRs reviewed by the pr-review skill.

Two-Phase Workflow

1. Title & Description Review - Verify PR metadata matches implementation
2. Code Review - Review code for best practices and potential issues

🚨 CRITICAL RULES

1. NEVER Approve or Request Changes

AI agents must NEVER use --approve or --request-changes flags.

| Action | Allowed? | Why | |--------|----------|-----| | gh pr review --approve | ❌ NEVER | Approval is a human decision | | gh pr review --request-changes | ❌ NEVER | Blocking PRs is a human decision |

2. NEVER Post Comments Directly

This skill is ANALYSIS ONLY. Never post comments using gh commands.

| Action | Allowed? | Why | |--------|----------|-----| | gh pr review --comment | ❌ NEVER | Review-PR.ps1 handles posting via scripts | | gh pr comment | ❌ NEVER | Review-PR.ps1 handles posting via scripts | | Analyze and report findings | ✅ YES | This is the skill's purpose |

Correct workflow:

1. This skill: Analyze PR, produce findings and write to pr-finalize-summary.md
2. Human-controlled follow-up: PR finalization is not part of the automated Review-PR.ps1 flow. Only post or use the summary when a user explicitly asks for PR finalization.

Only humans control when comments are posted. Your job is to analyze and present findings.

Phase 1: Title & Description

Core Principle: Preserve Quality

Review existing description BEFORE suggesting changes. Many PR authors write excellent, detailed descriptions. Your job is to:

1. Evaluate first - Is the existing description good? Better than a template?
2. Preserve quality - Don't replace a thorough description with a generic template
3. Enhance, don't replace - Add missing required elements (NOTE block, issue links) without rewriting good content
4. Only rewrite if needed - When description is stale, inaccurate, or missing key information

Usage

# Get current state (no local checkout required)
gh pr view XXXXX --json title,body
gh pr view XXXXX --json files --jq '.files[].path'

# Review commit messages (helpful for squash/merge commit quality)
gh pr view XXXXX --json commits --jq '.commits[].messageHeadline'

# Review actual code changes
gh pr diff XXXXX

# Optional: if the PR branch is checked out locally
git diff origin/main...HEAD

Evaluation Workflow

Step 1: Review Existing Description Quality

Before suggesting changes, evaluate the current description:

| Quality Indicator | Look For | |-------------------|----------| | Structure | Clear sections, headers, organized flow | | Technical depth | File-by-file changes, specific code references | | Scanability | Easy to find what changed and where | | Accuracy | Matches actual diff - not stale or incorrect | | Completeness | Platforms, breaking changes, testing info |

Step 2: Compare to Template

Ask: "Is the existing description better than what my template would produce?"

• If YES: Keep existing, only add missing required elements
• If NO: Suggest improvements or replacement

Step 3: Produce Output

• Recommended PR title (if change needed)
• Assessment of existing description
• Specific additions needed (e.g., "Add NOTE block at top")
• Only full replacement if description is inadequate

Title Requirements

The title becomes the commit message headline. Make it searchable and informative.

| Requirement | Good | Bad | |-------------|------|-----| | Platform prefix (if specific) | [iOS] Fix Shell back button | Fix Shell back button | | Describes behavior, not issue | [iOS] SafeArea: Return Empty for non-ISafeAreaView views | Fix #23892 | | Captures the "what" | Return Empty for non-ISafeAreaView | Fix SafeArea bug | | Notes model change if applicable | (opt-in model) | (omitted) | | No noise prefixes | [iOS] Fix... | [PR agent] Fix... |

Title Formula

[Platform] Component: What changed (model change if any)

Examples:

• [iOS] SafeArea: Return Empty for non-ISafeAreaView views (opt-in model)
• [Android] CollectionView: Fix scroll position reset on item update
• [Windows] Shell: Use NavigationView instead of custom flyout

Description Requirements

PR description should:

1. Include the base sections from .github/PULL_REQUEST_TEMPLATE.md ("Description of Change" and "Issues Fixed"). The skill adds additional structured fields (Root cause, Fix, Key insight, etc.) as recommended enhancements for better agent context.
2. Match the actual implementation

### Description of Change
[Must match actual implementation]

### Issues Fixed
Fixes #XXXXX

Content for Future Agents

The title and description become the commit message. Future agents searching git history will use this to understand:

• What changed and why
• What patterns to follow or avoid
• How this change affects related code

Required Elements for Agent Success

| Element | Purpose | Example | |---------|---------|---------| | Root cause | Why the bug occurred | "Non-ISafeAreaView views falling through to return baseSafeArea" | | Fix approach | What the code now does | "Return SafeAreaPadding.Empty for views without interface" | | Philosophy/model change | If behavior model changed | "Before: opt-out. After: opt-in via interface" | | Key interfaces/types | Types agents need to know | "ISafeAreaView, ISafeAreaView2 = opt-in contract" | | What NOT to do | Failed approaches to avoid | "Don't use Element type in Platform layer" | | Architectural constraints | Layer boundaries, type availability | "Platform layer cannot reference Controls types" | | Edge cases | Known limitations or risks | "Legacy layouts are [Obsolete], custom views need interface" |

"What NOT to Do" Section (Critical)

When try-fix or debugging revealed failed approaches, document them:

### What NOT to Do (for future agents)

- ❌ **Don't use [Type] in [Layer]** - [Why it fails]
- ❌ **Don't use [Pattern]** - [Why it's brittle/wrong]
- ❌ **Don't [Approach]** - [Why it doesn't work]

This prevents future agents from repeating failed experiments.

Philosophy/Model Changes

When a fix changes the behavioral model (not just fixing a bug), call it out explicitly:

**This is a philosophy change:**
- **Before:** [Old behavior model]
- **After:** [New behavior model]

Example: "Before: Safe area applied by default (opt-out). After: Only views implementing ISafeAreaView get safe area (opt-in)."

Common Issues

| Problem | Cause | Solution | |---------|-------|----------| | Description doesn't match code | Implementation changed during review | Update description to match actual diff | | Missing root cause | Author focused on "what" not "why" | Add root cause from issue/analysis | | References wrong approach | Started with A, switched to B | Update to describe final approach | | Missing NOTE block | Author didn't use template | Prepend NOTE block, keep rest | | Good description replaced | Agent used template blindly | Evaluate existing quality first |

Output Format

When Existing Description is Good

## PR #XXXXX Finalization Review

### ✅ Title: [Good / Needs Update]
**Current:** "Existing title"
**Recommended:** "[Platform] Improved title" (if needed)

### ✅ Description: Excellent - Keep As-Is

**Quality assessment:**
- Structure: ✅ Clear sections with headers
- Technical depth: ✅ File-by-file breakdown
- Accuracy: ✅ Matches implementation
- Completeness: ✅ Platforms, breaking changes noted

**Only addition needed:**
- ❌ Missing NOTE block - prepend to top

**Action:** Add NOTE block, preserve everything else.

When Description Needs Rewrite

Use structured template only when existing description is inadequate:

### Root Cause

[Why the bug occurred - be specific about the code path]

### Description of Change

[What the code now does]

**This is a philosophy change:** (if applicable)
- **Before:** [Old model]
- **After:** [New model]

[Cross-platform alignment notes if relevant]

### Key Technical Details

**[Relevant interfaces/types]:**
- `InterfaceA` - [What it does]
- `InterfaceB` - [What it does]

**[Category] that [work/don't work]:**
- List of types/views affected

### What NOT to Do (for future agents)

- ❌ **Don't [approach 1]** - [Why it fails]
- ❌ **Don't [approach 2]** - [Why it's wrong]
- ❌ **Don't [approach 3]** - [Constraint that prevents it]

### Edge Cases

| Scenario | Risk | Mitigation |
|----------|------|------------|
| [Case 1] | Low/Medium/High | [How to handle] |
| [Case 2] | Low/Medium/High | [How to handle] |

### Issues Fixed

Fixes #XXXXX

### Platforms Tested

- [x] iOS
- [x] Android
- [ ] Windows
- [ ] Mac

Quality Comparison Examples

Good Existing Description (KEEP)

## Changes Made

### 1. **PickerHandler.iOS.cs** - MacCatalyst-specific improvements

#### Added UIAlertController instance field
- Declared `UIAlertController? pickerController` as instance field...

#### Improved picker dismiss logic
- Moved picker dismiss logic from event handler to "Done" button action
- Removed `EditingDidEnd` event handler causing duplicate dismiss calls

## Platforms Affected
- **MacCatalyst** (primary)
- iOS (no behavior changes, shared code)

## Breaking Changes
None

Verdict: Excellent - file-by-file breakdown, specific changes, platforms, breaking changes. Keep it.

Poor Existing Description (REWRITE)

Fixed the issue mentioned in #30897

Verdict: Inadequate - no detail on what changed. Use template.

Phase 2: Code Review

After verifying title/description, perform a code review to catch best practice violations and potential issues before merge.

Review Focus Areas

When reviewing code changes, focus on:

1. Code quality and maintainability - Clean code, good naming, appropriate abstractions
2. Error handling and edge cases - Null checks, exception handling, boundary conditions
3. Performance implications - Unnecessary allocations, N+1 queries, blocking calls
4. Platform-specific concerns - iOS/Android/Windows differences, platform APIs
5. Breaking changes - API changes, behavior changes that affect existing code

How to Review

# Get the PR diff
gh pr diff XXXXX

# Review specific files
gh pr diff XXXXX -- path/to/file.cs

Output Format

## Code Review Findings

### 🔴 Critical Issues

**[Issue Title]**
- **File:** [path/to/file.cs]
- **Problem:** [Description]
- **Recommendation:** [Code fix or approach]

### 🟡 Suggestions

- [Suggestion 1]
- [Suggestion 2]

### ✅ Looks Good

- [Positive observation 1]
- [Positive observation 2]

🚨 CRITICAL: Do NOT Post Comments Directly

The pr-finalize skill is ANALYSIS ONLY. Never post comments using gh pr review or gh pr comment.

| Action | Allowed? | Why | |--------|----------|-----| | gh pr review --comment | ❌ NEVER | Review-PR.ps1 handles posting via scripts | | gh pr comment | ❌ NEVER | Review-PR.ps1 handles posting via scripts | | Analyze and report findings | ✅ YES | This is the skill's purpose |

Workflow:

1. This skill: Analyze PR, produce findings and write to pr-finalize-summary.md
2. Human-controlled follow-up: PR finalization is not part of the automated Review-PR.ps1 flow. Only post or use the summary when a user explicitly asks for PR finalization.

The user controls when comments are posted. Your job is to analyze and present findings.

Complete Example

See references/complete-example.md for a full agent-optimized PR description showing all elements above applied to a real SafeArea fix.