Improve verify-tests-fail-without-fix Skill

[kubaflo]

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description

This PR improves the verify-tests-fail-without-fix skill by fixing fork repository detection, improving error handling, and making full verification the recommended/default behavior.

Changes

1. Added git fetch origin for Accurate Diffs

• Fetches the base branch from origin before computing git diff
• Ensures accurate fix file detection even in fork repositories
• Prevents false negatives when base branch is stale locally

2. Improved Verification Mode Behavior

• Made full verification the primary/default mode - tests must FAIL without fix and PASS with fix
• Added -RequireFullVerification parameter - prevents silent fallback to verify-only mode
• Improved error handling - script now warns/errors if no fix files detected (instead of silently switching modes)
• Verify-only mode still available - runs when no fix files detected AND -RequireFullVerification not set
• Clearer expectations - user explicitly controls whether verify-only fallback is allowed

3. Improved Documentation

• Updated SKILL.md to reflect improved mode behavior
• Clarified requirements (fix files + test files for full verification)
• Updated mode selection table to show both modes clearly
• Added troubleshooting guidance for "no fix files detected" scenario

4. Better Error Handling

• Script exits early with actionable error if no fix files found AND -RequireFullVerification set
• Provides explicit solutions (use -FixFiles or -BaseBranch explicitly)
• Reduced ambiguity - user knows exactly what mode is running

Why These Changes?

Problem 1: Fork Repository Detection

When working in a fork, the base branch might not be up-to-date locally, causing git diff to produce inaccurate results. This led to:

• Missing fix files in the diff
• False "only test files" mode activation
• Incorrect verification results

Solution: Always fetch the base branch from origin before computing diff.

Problem 2: Confusing Auto-Detection

The previous auto-detection system was unclear:

• Silent fallback to "verify failure only" when no fix files detected
• Users didn't know which mode was running
• Made debugging harder

Solution:

• Add -RequireFullVerification parameter to make expectations explicit
• Error immediately if user expects full verification but no fix files found
• Maintain verify-only mode for legitimate test-first workflows

Problem 3: Silent Failures

When no fix files were detected (due to stale base branch or wrong base detection), the script would silently fall back to "verify failure only" mode, which wasn't the intended behavior.

Solution: Fail fast with clear error message and troubleshooting steps when -RequireFullVerification is set.

Testing

Verified the changes work correctly with:

• ✅ Fork repositories (fetch ensures accurate base branch)
• ✅ PRs with fix files + test files (full verification runs)
• ✅ Clear error when no fix files detected with -RequireFullVerification
• ✅ Verify-only mode still works when appropriate (without the flag)
• ✅ -RequireFullVerification parameter enforces strict mode

Example Usage

# Recommended for PR validation - ensures full verification
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform android -RequireFullVerification

# With explicit test filter
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform ios -TestFilter "Issue33356" -RequireFullVerification

# Override fix files or base branch if auto-detection fails
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform android -FixFiles @("src/Core/File.cs") -BaseBranch "main"

# Test-first workflow (verify tests fail before writing fix)
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform ios

Breaking Changes

⚠️ Behavior Change: The skill now requires explicit intent when fix files are not detected:

• Without -RequireFullVerification: Falls back to verify-only mode (tests only need to FAIL)
• With -RequireFullVerification: Errors immediately if no fix files found

Recommended usage: Always use -RequireFullVerification when validating PRs to ensure full verification (tests FAIL without fix, PASS with fix).

Migration: Update any automated scripts that call this skill to add -RequireFullVerification for PR validation workflows.

Related

• Part of improving PR verification workflows
• Complements the pr custom agent for better PR validation
• Ensures consistent behavior across fork and non-fork repositories

---

## Key Changes from Current Description

### Change 1: Section 2 Title and Content

**Current (inaccurate):**
```markdown
### 2. **Simplified Verification Mode**
- **Removed** "verify failure only" mode (when only test files exist)

Corrected:

### 2. **Improved Verification Mode Behavior**
- **Made full verification the primary/default mode** - tests must FAIL without fix and PASS with fix
- **Verify-only mode still available** - runs when no fix files detected AND `-RequireFullVerification` not set

Change 2: Breaking Changes Section

Current (inaccurate):

⚠️ **Behavior Change**: The skill no longer supports "verify failure only" mode.

Corrected:

⚠️ **Behavior Change**: The skill now requires explicit intent when fix files are not detected:
- **Without `-RequireFullVerification`**: Falls back to verify-only mode (tests only need to FAIL)
- **With `-RequireFullVerification`**: Errors immediately if no fix files found

Change 3: Added Test-First Workflow Example

Added:

# Test-first workflow (verify tests fail before writing fix)
pwsh .github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1 -Platform ios

This clarifies that verify-only mode is still useful for legitimate workflows.

---

Why This Correction Matters

Code Reality: The implementation shows verify-only mode still exists at lines 173-193:

if ($DetectedFixFiles.Count -eq 0) {
    Write-Host "║         VERIFY FAILURE ONLY MODE                          ║"
    # ... mode implementation
}

Impact of Inaccuracy:

• Future maintainers reading "removed" will be confused when they see the mode in code
• Users might think they can't use verify-only mode for test-first workflows
• Documentation doesn't match reality

Correction Approach:

• Acknowledge mode still exists
• Explain it's no longer the silent fallback (key improvement)
• Show it's still useful for test-first development
• Emphasize full verification is now recommended/default

---

[dotnet-policy-service]

Hey there @@kubaflo! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Copilot]

Pull request overview

This PR modifies the verify-tests-fail-without-fix skill to simplify its behavior and improve fork repository detection. The main changes include:

Changes:

• Removed the "verify failure only" mode (when only test files changed)
• Added git fetch origin to ensure base branch is up-to-date before computing diffs
• Added -RequireFullVerification parameter to enforce full verification mode
• Simplified documentation to focus on single-mode behavior (full verification only)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File	Description
.github/skills/verify-tests-fail-without-fix/scripts/verify-tests-fail.ps1	Removed ~100 lines of "verify failure only" mode logic, added git fetch for base branch, added RequireFullVerification parameter and error handling
.github/skills/verify-tests-fail-without-fix/SKILL.md	Updated documentation to remove references to dual-mode behavior and auto-detection table, simplified to describe only full verification mode

[PureWeen]

/rebase