diff --git a/plugins/dotnet/skills/csharp-refactoring/SKILL.md b/plugins/dotnet/skills/csharp-refactoring/SKILL.md index f9fef818..d7fbf2b3 100644 --- a/plugins/dotnet/skills/csharp-refactoring/SKILL.md +++ b/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: .` 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 diff --git a/tests/dotnet/csharp-refactoring/eval.yaml b/tests/dotnet/csharp-refactoring/eval.yaml index 70824c03..6af15f85 100644 --- a/tests/dotnet/csharp-refactoring/eval.yaml +++ b/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 = 'net8.0;net10.0'; + 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.*?)\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.*?)\n\s*\}', tests, re.S); + ordinary_caller = re.search( + r'RenderCurrency\s*\([^)]*\)\s*(?:=>|\{)(?P.*?)(?:;|\})', + 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 diff --git a/tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs b/tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs index aa446c10..1dd8eb31 100644 --- a/tests/dotnet/csharp-refactoring/src/Billing/Pricing.cs +++ b/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( diff --git a/tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs b/tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs index 56ef2938..46859777 100644 --- a/tests/dotnet/csharp-refactoring/tests/Billing.Tests/BillingTests.cs +++ b/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() {