Harden csharp refactoring evaluation

Clarify mixed-request classification, add a real shipped-wrapper caller, and strengthen deterministic fixture and delegation guards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8c45529b-2515-483d-9e51-e6c0b7cb6852

plugins/dotnet/skills/csharp-refactoring/SKILL.md

@@ -10,11 +10,12 @@ A refactor changes **structure**, never observable **behavior**. Do the edit wit
 then confirm behavior held with a build + the relevant tests. Keep the effort proportional to the change:
 a one-line local rename does not need the ceremony a public multi-targeted change does.
 
-## Mandatory gate: classify before searching or editing
+## Mandatory gate: classify before validation or editing
 
-Do this before reading project files, restoring, building, or making an edit. If the requested operation
-changes behavior or cannot preserve the relevant public/source contract, the correct result of this skill is a decisive handoff,
-not an implementation attempt:
+Read only enough repository context to classify the request. Do this before restoring, building, or
+making an edit. If the entire requested operation changes behavior or cannot preserve the relevant
+public/source contract, the correct result of this skill is a decisive handoff, not an implementation
+attempt:
 
 1. State: `Not a behavior-preserving refactor: <specific reason>.`
 2. State: `No files changed.`
@@ -24,12 +25,14 @@ not an implementation attempt:
 | Requested as a "refactor" | Classification and action |
 |---|---|
 | Framework or NuGet version change | **Upgrade.** Do not edit or validate the upgrade here; hand off to `dotnet-upgrade`. |
-| New capability, flag, endpoint, tier, or behavior | **Feature.** Do not implement it in this workflow. |
-| Threshold, rate, output, or bug-result change | **Behavior change.** Defer it unless separately authorized outside the refactor. |
-| Tighten or loosen a shipped/public nullable annotation | **Source-contract change.** Leave the declaration and API record unchanged. |
+| New capability, flag, endpoint, tier, or behavior | **Feature.** Do not implement it here; hand off to the repository's feature workflow. |
+| Threshold, rate, output, or bug-result change | **Behavior change.** Defer it and hand off to the repository's bug-fix or behavior-change workflow; still complete any clearly separable structural operation. |
+| Tighten or loosen a shipped/public nullable annotation | **Source-contract change.** Leave the declaration and API record unchanged; hand off to the repository's API-contract workflow. |
 
-For a mixed request, perform only a clearly separable structural operation and explicitly defer the
-behavior/contract change. Never modify tests to make an unauthorized behavior change appear preserved.
+The three-line stop response applies only when the entire request is outside behavior-preserving
+refactoring. For a mixed request, perform only a clearly separable structural operation and explicitly
+defer the behavior/contract change. Never state `No files changed.` after completing that structural
+operation, and never modify tests to make an unauthorized behavior change appear preserved.
 
 ## Work only in the current repository
 

tests/dotnet/csharp-refactoring/eval.yaml

@@ -76,6 +76,19 @@ stimuli:
             and 'ParseIntSetting(' not in ordinary_tests else 1)"
           expected_exit_code: 0
           timeout: 1m
+      - &fixture-integrity
+        type: run-command
+        config:
+          command: >-
+            python3 -c "from pathlib import Path; import re, sys;
+            expected = '<TargetFrameworks>net8.0;net10.0</TargetFrameworks>';
+            source_project = Path('src/Billing/Billing.csproj').read_text();
+            test_project = Path('tests/Billing.Tests/Billing.Tests.csproj').read_text();
+            tests = Path('tests/Billing.Tests/BillingTests.cs').read_text();
+            sys.exit(0 if expected in source_project and expected in test_project
+            and len(re.findall(r'\[Fact\]', tests)) >= 17 else 1)"
+          expected_exit_code: 0
+          timeout: 1m
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -116,6 +129,7 @@ stimuli:
         config:
           path: src/Billing/Coupons.g.cs.template
           value: private static decimal RateFor(
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -143,6 +157,8 @@ stimuli:
             match = re.search(r'class\s+ConfigReader\b[^{]*\{(?P<body>.*?)\n\}', text, re.S);
             body = match.group('body') if match else '';
             sys.exit(0 if 'ReadInt' in body and 'ReadBool' in body
+            and 'AppSettingsHelper.ParseIntSetting' in body
+            and 'AppSettingsHelper.ParseBoolSetting' in body
             and 'TryParse' not in body else 1)"
           expected_exit_code: 0
           timeout: 1m
@@ -150,6 +166,7 @@ stimuli:
         config:
           path: src/Billing/PublicAPI.Shipped.txt
           value: Billing.ConfigReader
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -178,6 +195,7 @@ stimuli:
         config:
           path: src/Billing/Pricing.cs
           value: ApplyTaxWrapper
+      - *fixture-integrity
       - type: prompt
     rubric:
       - Finds the true binding references to the wrapper before removing it, not just textual matches
@@ -206,6 +224,7 @@ stimuli:
         config:
           path: src/Billing/Pricing.cs
           value: class SilverPricing
+      - *fixture-integrity
       - type: prompt
     rubric:
       - Merges the two classes into a single type parameterized by name and rate, preserving each tier's values
@@ -277,6 +296,7 @@ stimuli:
         config:
           path: src/Billing/OrderProcessor.cs
           value: subtotal >= 100m
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -304,6 +324,7 @@ stimuli:
         config:
           path: tests/Billing.Tests/BillingTests.cs
           value: LoyaltyLevel = "gold"
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -327,6 +348,7 @@ stimuli:
         config:
           path: src/Billing/PublicAPI.Shipped.txt
           value: ParseBoolSetting(string? raw, bool fallback)
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -359,6 +381,7 @@ stimuli:
         config:
           path: src/Billing/CollisionExamples.cs
           value: DoStuff is shown in migration notes.
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -394,6 +417,7 @@ stimuli:
             sys.exit(0 if 'foreach' not in method and 'CalculateSubtotal(lines)' in method else 1)"
           expected_exit_code: 0
           timeout: 1m
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -425,6 +449,7 @@ stimuli:
         config:
           path: tests/Billing.Tests/BillingTests.cs
           value: PricingMath.ApplyDiscount
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -467,11 +492,18 @@ stimuli:
             compatibility_test = re.search(
             r'LegacyCurrencyFormatter_RemainsCompatible\s*\(\)\s*\{(?P<body>.*?)\n\s*\}',
             tests, re.S);
+            ordinary_caller = re.search(
+            r'RenderCurrency\s*\([^)]*\)\s*(?:=>|\{)(?P<body>.*?)(?:;|\})',
+            source, re.S);
             sys.exit(0 if shim and 'CurrencyFormatter.Format' in shim.group('body')
             and compatibility_test and 'LegacyCurrencyFormatter.FormatCurrency('
-            in compatibility_test.group('body') else 1)"
+            in compatibility_test.group('body')
+            and ordinary_caller and 'CurrencyFormatter.Format' in ordinary_caller.group('body')
+            and 'LegacyCurrencyFormatter.FormatCurrency' not in ordinary_caller.group('body')
+            else 1)"
           expected_exit_code: 0
           timeout: 1m
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -508,6 +540,7 @@ stimuli:
             and 'InvokeConfigured(\"RenderReceipt\", 12.5m)' in tests else 1)"
           expected_exit_code: 0
           timeout: 1m
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal
@@ -536,6 +569,7 @@ stimuli:
             sys.exit(0 if not re.search(r'\bLabel\s*\(', text) else 1)"
           expected_exit_code: 0
           timeout: 1m
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --framework net8.0 --verbosity normal
@@ -571,6 +605,7 @@ stimuli:
             and text.count('tier == \"silver\"') == 1 else 1)"
           expected_exit_code: 0
           timeout: 1m
+      - *fixture-integrity
       - type: run-command
         config:
           command: dotnet test Fixture.sln --verbosity normal

tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs

@@ -45,6 +45,8 @@ internal sealed class ReceiptRenderer
 {
     public string RenderDirect(decimal amount) => RenderReceipt(amount);
 
+    public string RenderCurrency(decimal amount) => LegacyCurrencyFormatter.FormatCurrency(amount);
+
     public string InvokeConfigured(string methodName, decimal amount)
     {
         var method = typeof(ReceiptRenderer).GetMethod(

tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs

@@ -84,6 +84,12 @@ public class BillingTests
         Assert.Equal("$12.50", LegacyCurrencyFormatter.FormatCurrency(12.5m));
     }
 
+    [Fact]
+    public void ReceiptRenderer_FormatsCurrency()
+    {
+        Assert.Equal("$12.50", new ReceiptRenderer().RenderCurrency(12.5m));
+    }
+
     [Fact]
     public void ReceiptRenderer_ConfiguredOldName_RemainsCompatible()
     {