diff --git a/.github/workflows/msbuild.yml b/.github/workflows/msbuild.yml index 04ca92912..92a0ff1fe 100644 --- a/.github/workflows/msbuild.yml +++ b/.github/workflows/msbuild.yml @@ -3,17 +3,23 @@ name: MSBuild Tasks on: workflow_dispatch: push: - paths-ignore: - - '**/*' - - '!.github/workflows/msbuild.yml' - - '!src/**/*.csproj' - - '!src/**/*.cs' + paths: + - '.github/workflows/msbuild.yml' + - 'src/**/*.csproj' + - 'src/**/*.cs' + - 'src/**/*.targets' + - 'src/Refitter.MSBuild/README.md' + - 'docs/docfx_project/articles/msbuild.md' + - 'test/MSBuild/**' pull_request: - paths-ignore: - - '**/*' - - '!.github/workflows/msbuild.yml' - - '!src/**/*.csproj' - - '!src/**/*.cs' + paths: + - '.github/workflows/msbuild.yml' + - 'src/**/*.csproj' + - 'src/**/*.cs' + - 'src/**/*.targets' + - 'src/Refitter.MSBuild/README.md' + - 'docs/docfx_project/articles/msbuild.md' + - 'test/MSBuild/**' jobs: build: @@ -31,20 +37,58 @@ jobs: dotnet pack -c release ../../src/Refitter.MSBuild/Refitter.MSBuild.csproj -o . nuget add .\Refitter.MSBuild.1.0.0.nupkg -source . dotnet add package Refitter.MSBuild -s . - - name: πŸ› οΈ Build (default naming) + - name: πŸ› οΈ Build (auto-scan enabled) working-directory: test/MSBuild - run: dotnet build -v d -filelogger -p:RefitterIncludePatterns="petstore.refitter" - - name: πŸ§ͺ Build Output (default) + shell: pwsh + run: | + Remove-Item Generated -Force -Recurse -ErrorAction SilentlyContinue + dotnet build -c release -v d -filelogger '-flp:logfile=msbuild.log;verbosity=diagnostic' -p:RefitterIncludePatterns="petstore.refitter" + if ($LASTEXITCODE -ne 0) { + throw "dotnet build failed (auto-scan enabled, exit code $LASTEXITCODE)" + } + if (!(Test-Path "Generated\Petstore.cs")) { + throw "Expected Generated\Petstore.cs when RefitterAutoScan is enabled." + } + - name: πŸ§ͺ Build Output (auto-scan enabled) working-directory: test/MSBuild run: cat msbuild.log - - name: πŸ› οΈ Build (PreserveOriginal naming) + - name: πŸ› οΈ Build (explicit generate target) working-directory: test/MSBuild + shell: pwsh run: | dotnet clean -c release --nologo - dotnet build -c release -v d -filelogger -p:RefitterIncludePatterns="petstore-preserve-original.refitter" - - name: πŸ§ͺ Build Output (PreserveOriginal) + Remove-Item Generated -Force -Recurse -ErrorAction SilentlyContinue + dotnet build -c release -t:RefitterGenerate -v d -filelogger '-flp:logfile=msbuild-explicit.log;verbosity=diagnostic' -p:RefitterAutoScan=false -p:RefitterIncludePatterns="petstore.refitter" + if ($LASTEXITCODE -ne 0) { + throw "dotnet build failed (-t:RefitterGenerate, exit code $LASTEXITCODE)" + } + if (!(Test-Path "Generated\Petstore.cs")) { + throw "Expected Generated\Petstore.cs after dotnet build -t:RefitterGenerate." + } + if (!(Select-String -Path msbuild-explicit.log -Pattern "Starting RefitterGenerateTask" -Quiet)) { + throw "Explicit RefitterGenerate target did not execute RefitterGenerateTask." + } + - name: πŸ§ͺ Build Output (explicit generate target) working-directory: test/MSBuild - run: cat msbuild.log + run: cat msbuild-explicit.log + - name: πŸ› οΈ Build (auto-scan disabled) + working-directory: test/MSBuild + shell: pwsh + run: | + dotnet clean -c release --nologo + dotnet build -c release -v d -filelogger '-flp:logfile=msbuild-disabled.log;verbosity=diagnostic' -p:RefitterAutoScan=false + if ($LASTEXITCODE -ne 0) { + throw "dotnet build failed (auto-scan disabled, exit code $LASTEXITCODE)" + } + if (!(Test-Path "Generated\Petstore.cs")) { + throw "Expected previously generated code to remain available when RefitterAutoScan=false." + } + if (Select-String -Path msbuild-disabled.log -Pattern "Starting RefitterGenerateTask" -Quiet) { + throw "RefitterGenerateTask ran during a normal build even though RefitterAutoScan=false." + } + - name: πŸ§ͺ Build Output (auto-scan disabled) + working-directory: test/MSBuild + run: cat msbuild-disabled.log - name: πŸ—³οΈ Upload uses: actions/upload-artifact@v7 with: diff --git a/.squad/agents/bishop/history.md b/.squad/agents/bishop/history.md index a9ad623f9..d8bc4f2a9 100644 --- a/.squad/agents/bishop/history.md +++ b/.squad/agents/bishop/history.md @@ -18,3 +18,13 @@ - **Issue #1025 OpenAPI Parser Migration (2026-04-20):** Added silent behavioral change section to breaking-changes guide documenting Microsoft.OpenApi.Readers upgrade (1.x β†’ 3.x). Clarifies that users will see different generated code due to improved parser schema interpretation (nullability, discriminators, refs, Swagger 2.0). Adds actionable migration steps: regenerate + review diff + test. Includes checklist item and evidence links (PR #907, #945). Commit: ccf293e7. Closes #1025. - **PR #1067 Doc Closure Review (2026-04-21):** Corrected stale source-generator docs that still described disk output, added explicit `Refit`/`Refit.HttpClientFactory` consumer guidance, clarified `RefitterIncludePatterns` is exact-match only, and reframed #1025 as migration mitigation rather than a fully proven behavioral fix. If PR wording mentions #1025 as fully fixed, it should be narrowed to "documents migration guidance for #1025." - **Issue #1083 communication triage (2026-05-01T14:04:19.681+02:00):** Reproduced the reporter's case against the current CLI: the schema name LookUpErnResponse. yields an empty internal partial class declaration and a Task<> return type. Current README/docfx wording only promises identifier sanitization in the propertyNamingPolicy / contract-property context, so this should be framed as a real product bug with docs that are technically correct but easy to over-read. + +## 2026-05-29T15:07:47 - Scribe: Issue #1094 Cross-Agent Update + +**Session:** commit-grouping-decision +**Context:** Dallas completed product implementation and created logical commits for issue #1094 (MSBuild RefitterAutoScan gating with explicit target fallback). + +**Team Status:** +- Lambert validated implementation as correct +- Decision recorded: split product changes into tooling/CI and documentation commits; housekeeping cleanup (stale automation + Squad bookkeeping) in separate commit with no co-author trailer +- Branch ready for review after Squad documentation completed diff --git a/.squad/agents/dallas/history.md b/.squad/agents/dallas/history.md index c68616941..66267759f 100644 --- a/.squad/agents/dallas/history.md +++ b/.squad/agents/dallas/history.md @@ -16,6 +16,10 @@ Parker, Lambert, and Dallas completed issue #1083 resolution: ## Learnings +- **2026-05-29T15:07:47.442+02:00 commit-grouping hygiene:** when issue work is already implemented, keep product commits split by behavior lane (tooling or CI first, docs second) and move stale automation artifacts like `.squad\commit-msg.txt` plus Squad bookkeeping into a final housekeeping commit so review stays focused and the branch ends clean. + +- **2026-05-29T14:24:16.307+02:00 issue #1094 tooling contract:** `src\Refitter.MSBuild\Refitter.MSBuild.targets` should keep `RefitterGenerate` explicitly invokable and move the automatic build hook into a separate target gated by `RefitterAutoScan`; with `RefitterAutoScan=false`, clean builds need an earlier `dotnet build -t:RefitterGenerate` pass so subsequent normal builds can compile the already-generated `.cs` files without re-running Refitter. + - **2026-05-01T14:34:56.630+02:00 issue #1083 tooling follow-up:** the dotted-schema-name fix is a core generator concern, so CLI/MSBuild wiring and README help text do not need parity changes; the only adjacent tooling work worth landing is source-generator regression coverage so compile-time generation proves the same sanitized DTO/return-type behavior as the CLI path. - **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec tooling verdict:** `test\economic.refitter` parses correctly and resolves both relative `openApiPaths`; CLI/MSBuild/source-generator all reach core multi-document merge, where duplicate equivalent schemas (`Error`, then `ProblemDetails`) trigger the fail-fast merge path before validation, so `--skip-validation` cannot help. @@ -85,4 +89,26 @@ Parker, Lambert, and Dallas completed issue #1083 resolution: - Root cause confirmed in `Refitter.Core` not tooling; `--skip-validation` cannot help because merge happens before validation. - Proposal: primary fix belongs in core merge semantics (allow duplicate paths/schemas when semantically equivalent); do not split `.refitter` into multiple generation runs. - Tooling follow-up (post-core-fix): improve CLI error guidance for merge failures, add `openApiPaths` regression coverage, add relative-path resolution tests, add merge-failure diagnostic tests. +## 2026-05-29T14:24:16 - Issue #1094: RefitterAutoScan MSBuild Implementation + +**Session:** issue-1094-real-implementation +**Status:** COMPLETE +**Approval:** Lambert validated implementation as correct and complete. + +**Work:** +- Edited 4 files for RefitterAutoScan MSBuild gating behavior +- Local validation: auto-scan enabled, explicit target with disable flag, normal build without task +- Implementation matches decision requirements + +**Outcome:** Ready for merge + +## 2026-05-29T15:07:47 - Scribe: Issue #1094 Squad Documentation + +**Session:** commit-grouping-decision +**Context:** Dallas finalized commit grouping with decision to split product changes into two commits and housekeeping cleanup (no co-author trailer). +**Squad Update:** +- Decision recorded: two product commits (tooling/CI and docs) plus one housekeeping commit for stale automation cleanup +- Inbox merged into decisions.md with commit grouping rationale +- Orchestration and session logs created +- Cross-agent history updated for Bishop and Lambert diff --git a/.squad/agents/lambert/history-archive.md b/.squad/agents/lambert/history-archive.md new file mode 100644 index 000000000..7c57c11f8 --- /dev/null +++ b/.squad/agents/lambert/history-archive.md @@ -0,0 +1,147 @@ +# Lambert History Archive + +## Pre-2026-05-29 Summary + +### Issue #1083 Resolution (2026-05-01) +- Reproduced schema type-name generation bug with dotted schema names +- Implemented regression test in Issue1083_SchemaTypeNameSanitizationTests +- Validated fix produces sanitized DTO names and compilable code +- Coverage: inline Swagger 2 fixture with LookUpErnResponse. schema key + +### Audit and Blocker Coverage (2026-04-25 to 2026-04-29) +- Narrowed P1 reproducible set to #1028, #1029, #1033, #1041, #1043 +- Stabilized CLI help output tests with semantic assertions +- Provided RefitterGenerateTask coverage analysis (91.86% line / 94.29% block) +- Validated Dallas's task coverage closure (100% line/block coverage achieved) +- Triaged e-conomic multi-spec merge failure - approved for fix +- Validated OpenApiDocumentFactory merge equivalence testing +- Confirmed 100% coverage achievement for OpenApiDocumentFactory + +### Testing Infrastructure +- Established baseline: 1918 tests passing at HEAD +- Identified environment-sensitive test surfaces (URL-based tests) +- Validated coverage measurement using Codecov command +- Confirmed Refitter.Core.dll, refitter.dll, RefitterMSBuild.dll coverage metrics + +### Key Learnings +- Semantic test assertions are safer than exact whitespace matching +- Multi-spec merge should handle equivalent duplicate schemas gracefully +- Coverage baselines should exclude environment-sensitive tests +- Clone APIs need careful validation for schema-type preservation + +## Full Historical Logs + +Below is the complete session history from 2026-04-16 to 2026-05-29: + +## Squad Session: Issue #1083 Resolution [2026-05-01T13.14.05.273Z] + +Parker, Lambert, and Dallas completed issue #1083 resolution: +- Core generation fix for schema type names +- Regression test coverage (inline scenario, collision/trailing-dot cases) +- Adjacent surface validation (CLI, MSBuild, Source Generator) +- All tests passing, generated code compiles successfully + +# Lambert History + +## Context + +- User: Christian Helle +- Product: Refitter generates C# REST API clients from OpenAPI specifications using Refit. +- Stack: .NET, Refit, NSwag, Source Generator, MSBuild, Microsoft OpenAPI.NET + +## Learnings + +- **2026-05-01T14:34:56.630+02:00 issue #1083 regression contract:** Kept coverage isolated in `Issue1083_SchemaTypeNameSanitizationTests` with a 1-endpoint inline Swagger 2 fixture. The durable assertions are: no blank `partial class`, no `Task<>`, the sanitized DTO name `LookUpErnResponse` appears in both the contract and interface signature, and generated code must compile. + +- **2026-05-01T14:04:19.681+02:00 issue #1083 repro:** Current HEAD reproduces issue #1083 with both the real Revenue schema name `LookUpErnResponse.` and a 1-endpoint minimal Swagger 2 fixture. Refitter emits `Task<> LookUpERN(...)` plus `partial class` with a blank identifier, and isolated compile checks fail with `CS1001 Identifier expected`. + +- **2026-05-01T14:04:19.681+02:00 issue #1083 coverage gap:** Existing identifier/property-name coverage (`PropertyNamingPolicyTests`, `PR1064BlockerRegressions`, `IdentifierUtilsTests`) stays green because it exercises property/parameter sanitization, not schema/type-name generation. The durable regression contract is a new scenario test around a dotted schema key plus a compile-backed assertion, with optional helper-level identifier assertions only if the fix routes through shared identifier utilities. + +- **2026-04-28T15:21:48.369+02:00 e-conomic OpenApiPaths regression coverage:** Added intended compile-backed regression coverage for the real `economic-products.json` + `economic-webhooks.json` inputs through `RefitGeneratorSettings.OpenApiPaths`. Current HEAD fails before code generation with duplicate schema `Error`, so the non-empty/buildable assertions become active evidence once the merge fix lands. + +- **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec repro:** `test\economic.refitter` fails during generation, not compilation, because `economic-products.json` and `economic-webhooks.json` both define identical `Error` and `ProblemDetails` component schemas; current merge logic throws on the first duplicate schema key (`Error`). Regression coverage should distinguish identical shared schemas from conflicting schema collisions. + +- Team initialized on 2026-04-16. +- **2026-04-28T12:02:17.298+02:00 issue #1045 evidence split:** Current HEAD already covers `.refitter` `openApiPaths` validation/execution and direct `RefitGenerator.CreateAsync(settings)` multi-spec usage. The reliable tester posture is to separate the CLI/settings-file claim from the direct-core-consumer claim, because they need different proofs and the current code/test matrix supports neither regression at HEAD. +- **2026-04-25 CLI help repro:** src\Refitter\Program.cs intentionally rewrites a no-argument invocation to --help, exits 0, and emits Spectre.Console.Cli help output. Tests in src\Refitter.Tests\GenerateCommandTests.cs should assert semantic help markers like usage, sections, and option names rather than exact formatter-driven spacing. +- **2026-04-25 Linux help-test follow-up:** GitHub Actions on Ubuntu still showed the semantic help text, but the raw redirected Spectre output did not satisfy the single regex assertion. The safe regression contract is to normalize console control sequences/line endings first and then assert semantic help markers (`USAGE`, usage text, sections, known option names). +- **PR #1064 / #1057 testing pattern:** When blocker work is in flux, Lambert's safest lane is minimal repro specs plus compilation gates, then focused test reruns once the implementing lane lands. +- **2026-04-26 baseline scan:** The current local validation baseline is green on `dotnet build -c Release src\Refitter.slnx --no-restore`, `dotnet test -c Release src\Refitter.slnx --no-build`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`; the solution test run passed 1894 tests with 0 failures. +- **2026-04-26 coverage baseline:** The existing repo coverage lane is the Codecov command from `.github\workflows\codecov.yml`: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`. That run passed 1848 tests and reported module coverage of `Refitter.Core.dll` 94.65% line / 96.04% block, `refitter.dll` 97.10% / 99.11%, `Refitter.MSBuild.dll` 100% / 100%, plus `.test-work` `RuntimeProof.dll` at 80.93% / 71.58% from generated proof artifacts. +- **2026-04-26 low-risk coverage targets:** The safest real source files to extend without behavior changes are `src\Refitter.Core\ContractTypeSuffixApplier.cs`, `src\Refitter.Core\SchemaCleaner.cs`, `src\Refitter.Core\CSharpClientGeneratorFactory.cs`, `src\Refitter.Core\XmlDocumentationGenerator.cs`, `src\Refitter.Core\IdentifierUtils.cs`, and `src\Refitter\Settings.cs`, because they already have focused tests nearby (`CoverageGapTests`, `SchemaCleanerTests`, `CSharpClientGeneratorFactoryTests`, `XmlDocumentationGeneratorTests`, `IdentifierUtilsTests`, `SettingsTests`) and remaining gaps are narrow branch cases. +- **2026-04-26 environment-sensitive tests:** External URL coverage still lives in `src\Refitter.Tests\OpenApiDocumentFactoryTests.cs`, `src\Refitter.Tests\SwaggerPetstoreTests.cs`, `src\Refitter.Tests\SwaggerPetstoreApizrTests.cs`, and `src\Refitter.Tests\Examples\OpenApiUrlTests.cs`; these should stay out of flaky-baseline conclusions during cleanup because they depend on live network access. + +## Core Context + +- **2026-04-17 release compatibility audit:** Confirmed two real breaking changes from 1.7.3 β†’ HEAD: the silent `.refitter` rename from `generateAuthenticationHeader` to `authenticationHeaderStyle`, and the source generator move from disk-written `.g.cs` files to Roslyn `AddSource()` output. +- **2026-04-18 P1 audit verification:** Validated ten high-priority issues and one partial, with the sharpest failure patterns in serializer-context regex parsing, identifier sanitization, dynamic querystring self-assignment, CLI precedence, and null content handling. +- **2026-04-20 PR #1064 blocker coverage:** Built regression tests around suffix-target collisions, multipart deduplication on sanitized identifiers, and keyword/title handling; later confirmed the blocker suite green once fixes landed. +- **2026-04-20 remaining P1 worktree audit:** Confirmed the tooling path for `GeneratedFile:` markers, flagged the netstandard build break and missing polymorphism/runtime proof for #1017, and kept #1024/#1025 open pending package and smoke-test evidence. + +## 2026-04-25: Remaining Audit Repro Pass + +- Narrowed the current-HEAD reproducible set to **#1028, #1029 (partial), #1033, #1041 (partial), and #1043**. +- Confirmed **#1032, #1042, #1045, and #1047** as validation-only or fixed-at-HEAD candidates unless stronger failing repros appear. +- Found no current-HEAD repro for **#1034, #1039, and #1056** in the initial tester pass. + +## 2026-04-25: Core Blocker-Test Lane + +- Ash's rejection kept **#1034** and **#1039** open and initially routed Lambert toward blocker-test coverage for the remaining failures. +- Dallas's later revisions shifted Lambert's lane from "write the first blocker tests" to reconciling blocker expectations against the landed merge and grouped-query behavior. +- After Dallas lockout, Lambert owned the final blocker-test revision for **#1034**, added explicit collision coverage, and reconciled the Issue1039_DynamicQuerystringMutationTests expectation drift. +- Ash still rejected that proof pass because the Swagger 2 definition-collision lane was not isolated cleanly enough, which moved the final narrow revision to Ripley. + +## 2026-04-25: CLI Help Output Test Stabilization + +- Reproduced the no-argument CLI path and confirmed the product behavior is correct. +- The durable test contract is semantic Spectre.Console.Cli help assertions, not exact whitespace/layout matching. +- Validation reported green for the release Refitter.Tests run, a focused rerun of Program_Main_Should_Show_Help_When_Invoked_Without_Arguments, and format verification. + +## 2026-04-25: Linux Help Output Fix Landed + +- Dallas's Ubuntu log analysis proved the failure was raw ANSI/wrapping noise from Spectre.Console help output rather than a CLI product bug. +- Lambert changed only src\Refitter.Tests\GenerateCommandTests.cs, normalizing redirected console output and asserting semantic help markers instead of formatter-specific layout. +- Reported final validation: dotnet build -c Release src\Refitter.slnx, dotnet test -c Release src\Refitter.slnx, and dotnet format --verify-no-changes src\Refitter.slnx. + +## 2026-04-25: RefitterGenerateTask coverage gap analysis + +- Coverage evidence from `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage-all.xml` leaves `src\Refitter.MSBuild\RefitterGenerateTask.cs` at **91.86% line / 94.29% block** coverage. +- Exact uncovered lanes are: `TryExecuteRefitter()` exception handling (lines 143-147), missing bundled CLI handling in `StartProcess()` (171-173), unresolved package-folder / co-located CLI / final-first-bundle fallbacks in `ResolveRefitterDll()` (304, 344-348, 351-353), the `<1000 ms` arm of `FormatTimeout()` (357-361 partial), and the non-throwing `Log.LogErrorFromException(e)` path (370). +- Full `Refitter.Tests` coverage run still hit the known network-dependent failures (`IsHttp_Detects_Https_Protocol` and `Can_Build_Generated_Code_From_Url(...)`), but it produced the decisive per-function coverage report needed for Dallas. + +## 2026-04-25: RefitterGenerateTask coverage closure landed + +- Dallas used the isolated branch list to add only regression coverage in `src\Refitter.Tests\RefitterGenerateTaskTests.cs`; no product behavior changes were needed in `src\Refitter.MSBuild\RefitterGenerateTask.cs`. +- The landed test pass covered exception handling, missing bundled CLI failure, blank package folder handling, whitespace runtime probing, co-located and first-bundled fallback resolution, millisecond timeout formatting, and successful `LogErrorFromException` forwarding. +- Reported validation: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`, `dotnet build -c Release src\Refitter.slnx --no-restore`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. +- Reported end state: `src\Refitter.MSBuild\RefitterGenerateTask.cs` at 100% line coverage, 100% block coverage, and 0 partial functions. + +## 2026-04-26: Shared AI-slop cleanup context + +- Ripley's triage keeps docs/help drift as the first low-risk cleanup batch, followed by settings/spec-path normalization and the shared `GeneratedFile:` marker contract. +- Dallas has already landed commit `f6374210` (`docs: clarify source generator setup`) with narrow validation green, so the doc-drift lane is actively moving without disturbing the baseline. +- Keep using the local green baseline plus focused coverage-safe files as the main cleanup signal; live-URL tests remain environment-sensitive and should stay out of pass/fail conclusions. + +- **2026-04-26 final revalidation after cleanup landing:** Current HEAD is green again on the trusted loop: `dotnet restore src\Refitter.slnx`, `dotnet build -c Release src\Refitter.slnx --no-restore`, `dotnet test -c Release src\Refitter.slnx --no-build`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. The solution test pass now reports 1918 succeeded / 0 failed, and the trusted coverage lane (`dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`) passed 1862 / 1862 with `Refitter.Core.dll` 95.05% line / 96.47% block, `refitter.dll` 97.10% / 99.11%, `Refitter.MSBuild.dll` back to 100% / 100%, and generated `RuntimeProof.dll` unchanged at 80.93% / 71.58%. + +## 2026-04-28: e-conomic Multi-Spec Merge Failure Repro (APPROVED FOR FIX) +- CLI failure repro: `dotnet .\src\Refitter\bin\Release\net9.0\refitter.dll --settings-file test\economic.refitter --no-banner --no-logging --simple-output` +- Failure: `InvalidOperationException: Cannot merge OpenAPI documents because a duplicate schema 'Error' was found.` +- Root cause: `economic-products.json` and `economic-webhooks.json` both define identical `Error` and `ProblemDetails` component schemas; current merge throws on first duplicate. +- Approved test coverage plan: + 1. `OpenApiDocumentFactoryMergeTests`: add `Merge_With_Equivalent_Duplicate_Schema_Does_Not_Throw` test (OpenAPI 3 docs with identical schema key/body); preserve collision conflict tests + 2. Optional e-conomic-shaped regression for `Error` schema with descriptions, nullable properties, extensions, `additionalProperties: false` + 3. Compile-backed regression: two specs with identical error schema and distinct paths; assert generation succeeds and code builds + 4. Post-fix generated-code validation for `test\economic.refitter`; triage any post-merge generation failures separately +- Regression gates: existing merge-collision tests must continue passing + +- **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec merge final validation:** Release build, focused `OpenApiDocumentFactoryMergeTests` (15 tests), focused `Issue1016_MultiSpecSchemaMergeTests` (7 tests), full Release solution tests, format verification, and the real `test\economic.refitter` CLI generation all passed. The required CLI command generated `test\GeneratedCode\economic.cs` successfully from `economic-products.json` + `economic-webhooks.json` without duplicate schema merge failures. + +## 2026-04-29: OpenApiDocumentFactory 100% Coverage & Obsolete Clone Validation + +- **2026-04-28T19:19:33Z User Directive:** Get `src/Refitter.Core/OpenApiDocumentFactory.cs` to 100% coverage; it is one of the most important parts of the codebase and all possibilities should be tested thoroughly. +- **2026-04-29T08:41:29Z Decision:** Treat `NSwag.OpenApiDocument.Tags` and `NSwag.OpenApiDocument.Components.Schemas` as non-null collections inside `Merge()`, eliminating unreachable null-handling branches that blocked 100% coverage. +- **Validation Result:** Coverage command confirmed `OpenApiDocumentFactory` at 100% line/block coverage in `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage.cobertura.xml`. +- **Clone Obsolete API Validation:** Added focused `OpenApiDocumentFactoryMergeTests` covering Swagger 2/3 round-trip serialization, verifying parameterless `ToJson()` + `FromJsonAsync()` preserves `SchemaType` across versions and maintains merge-critical collections. +- **Outcome:** Obsolete `ToJson(SchemaType)` replacement confirmed safe by focused regression tests proving schema-type preservation and mixed-version merge correctness. + diff --git a/.squad/agents/lambert/history-full.md b/.squad/agents/lambert/history-full.md new file mode 100644 index 000000000..7c57c11f8 --- /dev/null +++ b/.squad/agents/lambert/history-full.md @@ -0,0 +1,147 @@ +# Lambert History Archive + +## Pre-2026-05-29 Summary + +### Issue #1083 Resolution (2026-05-01) +- Reproduced schema type-name generation bug with dotted schema names +- Implemented regression test in Issue1083_SchemaTypeNameSanitizationTests +- Validated fix produces sanitized DTO names and compilable code +- Coverage: inline Swagger 2 fixture with LookUpErnResponse. schema key + +### Audit and Blocker Coverage (2026-04-25 to 2026-04-29) +- Narrowed P1 reproducible set to #1028, #1029, #1033, #1041, #1043 +- Stabilized CLI help output tests with semantic assertions +- Provided RefitterGenerateTask coverage analysis (91.86% line / 94.29% block) +- Validated Dallas's task coverage closure (100% line/block coverage achieved) +- Triaged e-conomic multi-spec merge failure - approved for fix +- Validated OpenApiDocumentFactory merge equivalence testing +- Confirmed 100% coverage achievement for OpenApiDocumentFactory + +### Testing Infrastructure +- Established baseline: 1918 tests passing at HEAD +- Identified environment-sensitive test surfaces (URL-based tests) +- Validated coverage measurement using Codecov command +- Confirmed Refitter.Core.dll, refitter.dll, RefitterMSBuild.dll coverage metrics + +### Key Learnings +- Semantic test assertions are safer than exact whitespace matching +- Multi-spec merge should handle equivalent duplicate schemas gracefully +- Coverage baselines should exclude environment-sensitive tests +- Clone APIs need careful validation for schema-type preservation + +## Full Historical Logs + +Below is the complete session history from 2026-04-16 to 2026-05-29: + +## Squad Session: Issue #1083 Resolution [2026-05-01T13.14.05.273Z] + +Parker, Lambert, and Dallas completed issue #1083 resolution: +- Core generation fix for schema type names +- Regression test coverage (inline scenario, collision/trailing-dot cases) +- Adjacent surface validation (CLI, MSBuild, Source Generator) +- All tests passing, generated code compiles successfully + +# Lambert History + +## Context + +- User: Christian Helle +- Product: Refitter generates C# REST API clients from OpenAPI specifications using Refit. +- Stack: .NET, Refit, NSwag, Source Generator, MSBuild, Microsoft OpenAPI.NET + +## Learnings + +- **2026-05-01T14:34:56.630+02:00 issue #1083 regression contract:** Kept coverage isolated in `Issue1083_SchemaTypeNameSanitizationTests` with a 1-endpoint inline Swagger 2 fixture. The durable assertions are: no blank `partial class`, no `Task<>`, the sanitized DTO name `LookUpErnResponse` appears in both the contract and interface signature, and generated code must compile. + +- **2026-05-01T14:04:19.681+02:00 issue #1083 repro:** Current HEAD reproduces issue #1083 with both the real Revenue schema name `LookUpErnResponse.` and a 1-endpoint minimal Swagger 2 fixture. Refitter emits `Task<> LookUpERN(...)` plus `partial class` with a blank identifier, and isolated compile checks fail with `CS1001 Identifier expected`. + +- **2026-05-01T14:04:19.681+02:00 issue #1083 coverage gap:** Existing identifier/property-name coverage (`PropertyNamingPolicyTests`, `PR1064BlockerRegressions`, `IdentifierUtilsTests`) stays green because it exercises property/parameter sanitization, not schema/type-name generation. The durable regression contract is a new scenario test around a dotted schema key plus a compile-backed assertion, with optional helper-level identifier assertions only if the fix routes through shared identifier utilities. + +- **2026-04-28T15:21:48.369+02:00 e-conomic OpenApiPaths regression coverage:** Added intended compile-backed regression coverage for the real `economic-products.json` + `economic-webhooks.json` inputs through `RefitGeneratorSettings.OpenApiPaths`. Current HEAD fails before code generation with duplicate schema `Error`, so the non-empty/buildable assertions become active evidence once the merge fix lands. + +- **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec repro:** `test\economic.refitter` fails during generation, not compilation, because `economic-products.json` and `economic-webhooks.json` both define identical `Error` and `ProblemDetails` component schemas; current merge logic throws on the first duplicate schema key (`Error`). Regression coverage should distinguish identical shared schemas from conflicting schema collisions. + +- Team initialized on 2026-04-16. +- **2026-04-28T12:02:17.298+02:00 issue #1045 evidence split:** Current HEAD already covers `.refitter` `openApiPaths` validation/execution and direct `RefitGenerator.CreateAsync(settings)` multi-spec usage. The reliable tester posture is to separate the CLI/settings-file claim from the direct-core-consumer claim, because they need different proofs and the current code/test matrix supports neither regression at HEAD. +- **2026-04-25 CLI help repro:** src\Refitter\Program.cs intentionally rewrites a no-argument invocation to --help, exits 0, and emits Spectre.Console.Cli help output. Tests in src\Refitter.Tests\GenerateCommandTests.cs should assert semantic help markers like usage, sections, and option names rather than exact formatter-driven spacing. +- **2026-04-25 Linux help-test follow-up:** GitHub Actions on Ubuntu still showed the semantic help text, but the raw redirected Spectre output did not satisfy the single regex assertion. The safe regression contract is to normalize console control sequences/line endings first and then assert semantic help markers (`USAGE`, usage text, sections, known option names). +- **PR #1064 / #1057 testing pattern:** When blocker work is in flux, Lambert's safest lane is minimal repro specs plus compilation gates, then focused test reruns once the implementing lane lands. +- **2026-04-26 baseline scan:** The current local validation baseline is green on `dotnet build -c Release src\Refitter.slnx --no-restore`, `dotnet test -c Release src\Refitter.slnx --no-build`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`; the solution test run passed 1894 tests with 0 failures. +- **2026-04-26 coverage baseline:** The existing repo coverage lane is the Codecov command from `.github\workflows\codecov.yml`: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`. That run passed 1848 tests and reported module coverage of `Refitter.Core.dll` 94.65% line / 96.04% block, `refitter.dll` 97.10% / 99.11%, `Refitter.MSBuild.dll` 100% / 100%, plus `.test-work` `RuntimeProof.dll` at 80.93% / 71.58% from generated proof artifacts. +- **2026-04-26 low-risk coverage targets:** The safest real source files to extend without behavior changes are `src\Refitter.Core\ContractTypeSuffixApplier.cs`, `src\Refitter.Core\SchemaCleaner.cs`, `src\Refitter.Core\CSharpClientGeneratorFactory.cs`, `src\Refitter.Core\XmlDocumentationGenerator.cs`, `src\Refitter.Core\IdentifierUtils.cs`, and `src\Refitter\Settings.cs`, because they already have focused tests nearby (`CoverageGapTests`, `SchemaCleanerTests`, `CSharpClientGeneratorFactoryTests`, `XmlDocumentationGeneratorTests`, `IdentifierUtilsTests`, `SettingsTests`) and remaining gaps are narrow branch cases. +- **2026-04-26 environment-sensitive tests:** External URL coverage still lives in `src\Refitter.Tests\OpenApiDocumentFactoryTests.cs`, `src\Refitter.Tests\SwaggerPetstoreTests.cs`, `src\Refitter.Tests\SwaggerPetstoreApizrTests.cs`, and `src\Refitter.Tests\Examples\OpenApiUrlTests.cs`; these should stay out of flaky-baseline conclusions during cleanup because they depend on live network access. + +## Core Context + +- **2026-04-17 release compatibility audit:** Confirmed two real breaking changes from 1.7.3 β†’ HEAD: the silent `.refitter` rename from `generateAuthenticationHeader` to `authenticationHeaderStyle`, and the source generator move from disk-written `.g.cs` files to Roslyn `AddSource()` output. +- **2026-04-18 P1 audit verification:** Validated ten high-priority issues and one partial, with the sharpest failure patterns in serializer-context regex parsing, identifier sanitization, dynamic querystring self-assignment, CLI precedence, and null content handling. +- **2026-04-20 PR #1064 blocker coverage:** Built regression tests around suffix-target collisions, multipart deduplication on sanitized identifiers, and keyword/title handling; later confirmed the blocker suite green once fixes landed. +- **2026-04-20 remaining P1 worktree audit:** Confirmed the tooling path for `GeneratedFile:` markers, flagged the netstandard build break and missing polymorphism/runtime proof for #1017, and kept #1024/#1025 open pending package and smoke-test evidence. + +## 2026-04-25: Remaining Audit Repro Pass + +- Narrowed the current-HEAD reproducible set to **#1028, #1029 (partial), #1033, #1041 (partial), and #1043**. +- Confirmed **#1032, #1042, #1045, and #1047** as validation-only or fixed-at-HEAD candidates unless stronger failing repros appear. +- Found no current-HEAD repro for **#1034, #1039, and #1056** in the initial tester pass. + +## 2026-04-25: Core Blocker-Test Lane + +- Ash's rejection kept **#1034** and **#1039** open and initially routed Lambert toward blocker-test coverage for the remaining failures. +- Dallas's later revisions shifted Lambert's lane from "write the first blocker tests" to reconciling blocker expectations against the landed merge and grouped-query behavior. +- After Dallas lockout, Lambert owned the final blocker-test revision for **#1034**, added explicit collision coverage, and reconciled the Issue1039_DynamicQuerystringMutationTests expectation drift. +- Ash still rejected that proof pass because the Swagger 2 definition-collision lane was not isolated cleanly enough, which moved the final narrow revision to Ripley. + +## 2026-04-25: CLI Help Output Test Stabilization + +- Reproduced the no-argument CLI path and confirmed the product behavior is correct. +- The durable test contract is semantic Spectre.Console.Cli help assertions, not exact whitespace/layout matching. +- Validation reported green for the release Refitter.Tests run, a focused rerun of Program_Main_Should_Show_Help_When_Invoked_Without_Arguments, and format verification. + +## 2026-04-25: Linux Help Output Fix Landed + +- Dallas's Ubuntu log analysis proved the failure was raw ANSI/wrapping noise from Spectre.Console help output rather than a CLI product bug. +- Lambert changed only src\Refitter.Tests\GenerateCommandTests.cs, normalizing redirected console output and asserting semantic help markers instead of formatter-specific layout. +- Reported final validation: dotnet build -c Release src\Refitter.slnx, dotnet test -c Release src\Refitter.slnx, and dotnet format --verify-no-changes src\Refitter.slnx. + +## 2026-04-25: RefitterGenerateTask coverage gap analysis + +- Coverage evidence from `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage-all.xml` leaves `src\Refitter.MSBuild\RefitterGenerateTask.cs` at **91.86% line / 94.29% block** coverage. +- Exact uncovered lanes are: `TryExecuteRefitter()` exception handling (lines 143-147), missing bundled CLI handling in `StartProcess()` (171-173), unresolved package-folder / co-located CLI / final-first-bundle fallbacks in `ResolveRefitterDll()` (304, 344-348, 351-353), the `<1000 ms` arm of `FormatTimeout()` (357-361 partial), and the non-throwing `Log.LogErrorFromException(e)` path (370). +- Full `Refitter.Tests` coverage run still hit the known network-dependent failures (`IsHttp_Detects_Https_Protocol` and `Can_Build_Generated_Code_From_Url(...)`), but it produced the decisive per-function coverage report needed for Dallas. + +## 2026-04-25: RefitterGenerateTask coverage closure landed + +- Dallas used the isolated branch list to add only regression coverage in `src\Refitter.Tests\RefitterGenerateTaskTests.cs`; no product behavior changes were needed in `src\Refitter.MSBuild\RefitterGenerateTask.cs`. +- The landed test pass covered exception handling, missing bundled CLI failure, blank package folder handling, whitespace runtime probing, co-located and first-bundled fallback resolution, millisecond timeout formatting, and successful `LogErrorFromException` forwarding. +- Reported validation: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`, `dotnet build -c Release src\Refitter.slnx --no-restore`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. +- Reported end state: `src\Refitter.MSBuild\RefitterGenerateTask.cs` at 100% line coverage, 100% block coverage, and 0 partial functions. + +## 2026-04-26: Shared AI-slop cleanup context + +- Ripley's triage keeps docs/help drift as the first low-risk cleanup batch, followed by settings/spec-path normalization and the shared `GeneratedFile:` marker contract. +- Dallas has already landed commit `f6374210` (`docs: clarify source generator setup`) with narrow validation green, so the doc-drift lane is actively moving without disturbing the baseline. +- Keep using the local green baseline plus focused coverage-safe files as the main cleanup signal; live-URL tests remain environment-sensitive and should stay out of pass/fail conclusions. + +- **2026-04-26 final revalidation after cleanup landing:** Current HEAD is green again on the trusted loop: `dotnet restore src\Refitter.slnx`, `dotnet build -c Release src\Refitter.slnx --no-restore`, `dotnet test -c Release src\Refitter.slnx --no-build`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. The solution test pass now reports 1918 succeeded / 0 failed, and the trusted coverage lane (`dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`) passed 1862 / 1862 with `Refitter.Core.dll` 95.05% line / 96.47% block, `refitter.dll` 97.10% / 99.11%, `Refitter.MSBuild.dll` back to 100% / 100%, and generated `RuntimeProof.dll` unchanged at 80.93% / 71.58%. + +## 2026-04-28: e-conomic Multi-Spec Merge Failure Repro (APPROVED FOR FIX) +- CLI failure repro: `dotnet .\src\Refitter\bin\Release\net9.0\refitter.dll --settings-file test\economic.refitter --no-banner --no-logging --simple-output` +- Failure: `InvalidOperationException: Cannot merge OpenAPI documents because a duplicate schema 'Error' was found.` +- Root cause: `economic-products.json` and `economic-webhooks.json` both define identical `Error` and `ProblemDetails` component schemas; current merge throws on first duplicate. +- Approved test coverage plan: + 1. `OpenApiDocumentFactoryMergeTests`: add `Merge_With_Equivalent_Duplicate_Schema_Does_Not_Throw` test (OpenAPI 3 docs with identical schema key/body); preserve collision conflict tests + 2. Optional e-conomic-shaped regression for `Error` schema with descriptions, nullable properties, extensions, `additionalProperties: false` + 3. Compile-backed regression: two specs with identical error schema and distinct paths; assert generation succeeds and code builds + 4. Post-fix generated-code validation for `test\economic.refitter`; triage any post-merge generation failures separately +- Regression gates: existing merge-collision tests must continue passing + +- **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec merge final validation:** Release build, focused `OpenApiDocumentFactoryMergeTests` (15 tests), focused `Issue1016_MultiSpecSchemaMergeTests` (7 tests), full Release solution tests, format verification, and the real `test\economic.refitter` CLI generation all passed. The required CLI command generated `test\GeneratedCode\economic.cs` successfully from `economic-products.json` + `economic-webhooks.json` without duplicate schema merge failures. + +## 2026-04-29: OpenApiDocumentFactory 100% Coverage & Obsolete Clone Validation + +- **2026-04-28T19:19:33Z User Directive:** Get `src/Refitter.Core/OpenApiDocumentFactory.cs` to 100% coverage; it is one of the most important parts of the codebase and all possibilities should be tested thoroughly. +- **2026-04-29T08:41:29Z Decision:** Treat `NSwag.OpenApiDocument.Tags` and `NSwag.OpenApiDocument.Components.Schemas` as non-null collections inside `Merge()`, eliminating unreachable null-handling branches that blocked 100% coverage. +- **Validation Result:** Coverage command confirmed `OpenApiDocumentFactory` at 100% line/block coverage in `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage.cobertura.xml`. +- **Clone Obsolete API Validation:** Added focused `OpenApiDocumentFactoryMergeTests` covering Swagger 2/3 round-trip serialization, verifying parameterless `ToJson()` + `FromJsonAsync()` preserves `SchemaType` across versions and maintains merge-critical collections. +- **Outcome:** Obsolete `ToJson(SchemaType)` replacement confirmed safe by focused regression tests proving schema-type preservation and mixed-version merge correctness. + diff --git a/.squad/agents/lambert/history.md b/.squad/agents/lambert/history.md index d7c52d418..b1ede56de 100644 --- a/.squad/agents/lambert/history.md +++ b/.squad/agents/lambert/history.md @@ -1,111 +1,53 @@ -## Squad Session: Issue #1083 Resolution [2026-05-01T13.14.05.273Z] - -Parker, Lambert, and Dallas completed issue #1083 resolution: -- Core generation fix for schema type names -- Regression test coverage (inline scenario, collision/trailing-dot cases) -- Adjacent surface validation (CLI, MSBuild, Source Generator) -- All tests passing, generated code compiles successfully - # Lambert History -## Context - -- User: Christian Helle -- Product: Refitter generates C# REST API clients from OpenAPI specifications using Refit. -- Stack: .NET, Refit, NSwag, Source Generator, MSBuild, Microsoft OpenAPI.NET - -## Learnings - -- **2026-05-01T14:34:56.630+02:00 issue #1083 regression contract:** Kept coverage isolated in `Issue1083_SchemaTypeNameSanitizationTests` with a 1-endpoint inline Swagger 2 fixture. The durable assertions are: no blank `partial class`, no `Task<>`, the sanitized DTO name `LookUpErnResponse` appears in both the contract and interface signature, and generated code must compile. - -- **2026-05-01T14:04:19.681+02:00 issue #1083 repro:** Current HEAD reproduces issue #1083 with both the real Revenue schema name `LookUpErnResponse.` and a 1-endpoint minimal Swagger 2 fixture. Refitter emits `Task<> LookUpERN(...)` plus `partial class` with a blank identifier, and isolated compile checks fail with `CS1001 Identifier expected`. - -- **2026-05-01T14:04:19.681+02:00 issue #1083 coverage gap:** Existing identifier/property-name coverage (`PropertyNamingPolicyTests`, `PR1064BlockerRegressions`, `IdentifierUtilsTests`) stays green because it exercises property/parameter sanitization, not schema/type-name generation. The durable regression contract is a new scenario test around a dotted schema key plus a compile-backed assertion, with optional helper-level identifier assertions only if the fix routes through shared identifier utilities. +## Current Session: Issue #1094 - RefitterAutoScan (2026-05-29) -- **2026-04-28T15:21:48.369+02:00 e-conomic OpenApiPaths regression coverage:** Added intended compile-backed regression coverage for the real `economic-products.json` + `economic-webhooks.json` inputs through `RefitGeneratorSettings.OpenApiPaths`. Current HEAD fails before code generation with duplicate schema `Error`, so the non-empty/buildable assertions become active evidence once the merge fix lands. +### Role -- **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec repro:** `test\economic.refitter` fails during generation, not compilation, because `economic-products.json` and `economic-webhooks.json` both define identical `Error` and `ProblemDetails` component schemas; current merge logic throws on the first duplicate schema key (`Error`). Regression coverage should distinguish identical shared schemas from conflicting schema collisions. +- Validation: Reviewed implementation details and corrected documentation against code +- Status: Approved corrected documentation after Dallas's revision cycle -- Team initialized on 2026-04-16. -- **2026-04-28T12:02:17.298+02:00 issue #1045 evidence split:** Current HEAD already covers `.refitter` `openApiPaths` validation/execution and direct `RefitGenerator.CreateAsync(settings)` multi-spec usage. The reliable tester posture is to separate the CLI/settings-file claim from the direct-core-consumer claim, because they need different proofs and the current code/test matrix supports neither regression at HEAD. -- **2026-04-25 CLI help repro:** src\Refitter\Program.cs intentionally rewrites a no-argument invocation to --help, exits 0, and emits Spectre.Console.Cli help output. Tests in src\Refitter.Tests\GenerateCommandTests.cs should assert semantic help markers like usage, sections, and option names rather than exact formatter-driven spacing. -- **2026-04-25 Linux help-test follow-up:** GitHub Actions on Ubuntu still showed the semantic help text, but the raw redirected Spectre output did not satisfy the single regex assertion. The safe regression contract is to normalize console control sequences/line endings first and then assert semantic help markers (`USAGE`, usage text, sections, known option names). -- **PR #1064 / #1057 testing pattern:** When blocker work is in flux, Lambert's safest lane is minimal repro specs plus compilation gates, then focused test reruns once the implementing lane lands. -- **2026-04-26 baseline scan:** The current local validation baseline is green on `dotnet build -c Release src\Refitter.slnx --no-restore`, `dotnet test -c Release src\Refitter.slnx --no-build`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`; the solution test run passed 1894 tests with 0 failures. -- **2026-04-26 coverage baseline:** The existing repo coverage lane is the Codecov command from `.github\workflows\codecov.yml`: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`. That run passed 1848 tests and reported module coverage of `Refitter.Core.dll` 94.65% line / 96.04% block, `refitter.dll` 97.10% / 99.11%, `Refitter.MSBuild.dll` 100% / 100%, plus `.test-work` `RuntimeProof.dll` at 80.93% / 71.58% from generated proof artifacts. -- **2026-04-26 low-risk coverage targets:** The safest real source files to extend without behavior changes are `src\Refitter.Core\ContractTypeSuffixApplier.cs`, `src\Refitter.Core\SchemaCleaner.cs`, `src\Refitter.Core\CSharpClientGeneratorFactory.cs`, `src\Refitter.Core\XmlDocumentationGenerator.cs`, `src\Refitter.Core\IdentifierUtils.cs`, and `src\Refitter\Settings.cs`, because they already have focused tests nearby (`CoverageGapTests`, `SchemaCleanerTests`, `CSharpClientGeneratorFactoryTests`, `XmlDocumentationGeneratorTests`, `IdentifierUtilsTests`, `SettingsTests`) and remaining gaps are narrow branch cases. -- **2026-04-26 environment-sensitive tests:** External URL coverage still lives in `src\Refitter.Tests\OpenApiDocumentFactoryTests.cs`, `src\Refitter.Tests\SwaggerPetstoreTests.cs`, `src\Refitter.Tests\SwaggerPetstoreApizrTests.cs`, and `src\Refitter.Tests\Examples\OpenApiUrlTests.cs`; these should stay out of flaky-baseline conclusions during cleanup because they depend on live network access. +### Learning -## Core Context +- **2026-05-29T12:44:43.282+02:00 issue #1094 doc re-review:** The corrected MSBuild docs now match src\Refitter.MSBuild\Refitter.MSBuild.targets: RefitterGenerate stays as the explicit on-demand target, while _RefitterGenerateOnBuild is the BeforeTargets="CoreCompile" wrapper gated by RefitterAutoScan != false. -- **2026-04-17 release compatibility audit:** Confirmed two real breaking changes from 1.7.3 β†’ HEAD: the silent `.refitter` rename from `generateAuthenticationHeader` to `authenticationHeaderStyle`, and the source generator move from disk-written `.g.cs` files to Roslyn `AddSource()` output. -- **2026-04-18 P1 audit verification:** Validated ten high-priority issues and one partial, with the sharpest failure patterns in serializer-context regex parsing, identifier sanitization, dynamic querystring self-assignment, CLI precedence, and null content handling. -- **2026-04-20 PR #1064 blocker coverage:** Built regression tests around suffix-target collisions, multipart deduplication on sanitized identifiers, and keyword/title handling; later confirmed the blocker suite green once fixes landed. -- **2026-04-20 remaining P1 worktree audit:** Confirmed the tooling path for `GeneratedFile:` markers, flagged the netstandard build break and missing polymorphism/runtime proof for #1017, and kept #1024/#1025 open pending package and smoke-test evidence. +- **2026-05-29T12:44:43.282+02:00 issue #1094 validation:** src\Refitter.MSBuild\Refitter.MSBuild.targets now defaults RefitterAutoScan to rue, keeps RefitterGenerate directly callable, and gates normal-build execution through _RefitterGenerateOnBuild with BeforeTargets="CoreCompile" plus Condition="'$(RefitterAutoScan)' != 'false'". -## 2026-04-25: Remaining Audit Repro Pass +### Evidence -- Narrowed the current-HEAD reproducible set to **#1028, #1029 (partial), #1033, #1041 (partial), and #1043**. -- Confirmed **#1032, #1042, #1045, and #1047** as validation-only or fixed-at-HEAD candidates unless stronger failing repros appear. -- Found no current-HEAD repro for **#1034, #1039, and #1056** in the initial tester pass. +- Implementation in src\Refitter.MSBuild\Refitter.MSBuild.targets matches documented behavior +- Documentation in README.md and docfx article now accurately describes the feature +- No documentation-to-implementation mismatch remains -## 2026-04-25: Core Blocker-Test Lane - -- Ash's rejection kept **#1034** and **#1039** open and initially routed Lambert toward blocker-test coverage for the remaining failures. -- Dallas's later revisions shifted Lambert's lane from "write the first blocker tests" to reconciling blocker expectations against the landed merge and grouped-query behavior. -- After Dallas lockout, Lambert owned the final blocker-test revision for **#1034**, added explicit collision coverage, and reconciled the Issue1039_DynamicQuerystringMutationTests expectation drift. -- Ash still rejected that proof pass because the Swagger 2 definition-collision lane was not isolated cleanly enough, which moved the final narrow revision to Ripley. - -## 2026-04-25: CLI Help Output Test Stabilization - -- Reproduced the no-argument CLI path and confirmed the product behavior is correct. -- The durable test contract is semantic Spectre.Console.Cli help assertions, not exact whitespace/layout matching. -- Validation reported green for the release Refitter.Tests run, a focused rerun of Program_Main_Should_Show_Help_When_Invoked_Without_Arguments, and format verification. - -## 2026-04-25: Linux Help Output Fix Landed - -- Dallas's Ubuntu log analysis proved the failure was raw ANSI/wrapping noise from Spectre.Console help output rather than a CLI product bug. -- Lambert changed only src\Refitter.Tests\GenerateCommandTests.cs, normalizing redirected console output and asserting semantic help markers instead of formatter-specific layout. -- Reported final validation: dotnet build -c Release src\Refitter.slnx, dotnet test -c Release src\Refitter.slnx, and dotnet format --verify-no-changes src\Refitter.slnx. - -## 2026-04-25: RefitterGenerateTask coverage gap analysis +## Learnings -- Coverage evidence from `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage-all.xml` leaves `src\Refitter.MSBuild\RefitterGenerateTask.cs` at **91.86% line / 94.29% block** coverage. -- Exact uncovered lanes are: `TryExecuteRefitter()` exception handling (lines 143-147), missing bundled CLI handling in `StartProcess()` (171-173), unresolved package-folder / co-located CLI / final-first-bundle fallbacks in `ResolveRefitterDll()` (304, 344-348, 351-353), the `<1000 ms` arm of `FormatTimeout()` (357-361 partial), and the non-throwing `Log.LogErrorFromException(e)` path (370). -- Full `Refitter.Tests` coverage run still hit the known network-dependent failures (`IsHttp_Detects_Https_Protocol` and `Can_Build_Generated_Code_From_Url(...)`), but it produced the decisive per-function coverage report needed for Dallas. +- **2026-05-29T14:24:16.307+02:00 issue #1094 working-tree validation:** The real local diff is present in `.github\workflows\msbuild.yml`, `docs\docfx_project\articles\msbuild.md`, `src\Refitter.MSBuild\README.md`, and `src\Refitter.MSBuild\Refitter.MSBuild.targets`; local MSBuild validation confirmed `RefitterAutoScan` defaults to `true`, `dotnet build -t:RefitterGenerate -p:RefitterAutoScan=false` still runs generation, and normal builds with `RefitterAutoScan=false` skip `RefitterGenerateTask` while reusing generated code. -## 2026-04-25: RefitterGenerateTask coverage closure landed +--- -- Dallas used the isolated branch list to add only regression coverage in `src\Refitter.Tests\RefitterGenerateTaskTests.cs`; no product behavior changes were needed in `src\Refitter.MSBuild\RefitterGenerateTask.cs`. -- The landed test pass covered exception handling, missing bundled CLI failure, blank package folder handling, whitespace runtime probing, co-located and first-bundled fallback resolution, millisecond timeout formatting, and successful `LogErrorFromException` forwarding. -- Reported validation: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`, `dotnet build -c Release src\Refitter.slnx --no-restore`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. -- Reported end state: `src\Refitter.MSBuild\RefitterGenerateTask.cs` at 100% line coverage, 100% block coverage, and 0 partial functions. +## Archive -## 2026-04-26: Shared AI-slop cleanup context +For historical context on issues #1083, #1057, #1045, #1034, #1039, and earlier audit work, see history-archive.md. +## 2026-05-29T14:24:16 - Issue #1094: RefitterAutoScan Implementation Validation -- Ripley's triage keeps docs/help drift as the first low-risk cleanup batch, followed by settings/spec-path normalization and the shared `GeneratedFile:` marker contract. -- Dallas has already landed commit `f6374210` (`docs: clarify source generator setup`) with narrow validation green, so the doc-drift lane is actively moving without disturbing the baseline. -- Keep using the local green baseline plus focused coverage-safe files as the main cleanup signal; live-URL tests remain environment-sensitive and should stay out of pass/fail conclusions. +**Session:** issue-1094-real-implementation +**Status:** COMPLETE +**Role:** Code review and validation -- **2026-04-26 final revalidation after cleanup landing:** Current HEAD is green again on the trusted loop: `dotnet restore src\Refitter.slnx`, `dotnet build -c Release src\Refitter.slnx --no-restore`, `dotnet test -c Release src\Refitter.slnx --no-build`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. The solution test pass now reports 1918 succeeded / 0 failed, and the trusted coverage lane (`dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`) passed 1862 / 1862 with `Refitter.Core.dll` 95.05% line / 96.47% block, `refitter.dll` 97.10% / 99.11%, `Refitter.MSBuild.dll` back to 100% / 100%, and generated `RuntimeProof.dll` unchanged at 80.93% / 71.58%. +**Work:** +- Reviewed Dallas's actual working-tree diff (4 files) +- Re-ran full MSBuild validation flow: restore, build, test, format +- Verified implementation against requirements -## 2026-04-28: e-conomic Multi-Spec Merge Failure Repro (APPROVED FOR FIX) -- CLI failure repro: `dotnet .\src\Refitter\bin\Release\net9.0\refitter.dll --settings-file test\economic.refitter --no-banner --no-logging --simple-output` -- Failure: `InvalidOperationException: Cannot merge OpenAPI documents because a duplicate schema 'Error' was found.` -- Root cause: `economic-products.json` and `economic-webhooks.json` both define identical `Error` and `ProblemDetails` component schemas; current merge throws on first duplicate. -- Approved test coverage plan: - 1. `OpenApiDocumentFactoryMergeTests`: add `Merge_With_Equivalent_Duplicate_Schema_Does_Not_Throw` test (OpenAPI 3 docs with identical schema key/body); preserve collision conflict tests - 2. Optional e-conomic-shaped regression for `Error` schema with descriptions, nullable properties, extensions, `additionalProperties: false` - 3. Compile-backed regression: two specs with identical error schema and distinct paths; assert generation succeeds and code builds - 4. Post-fix generated-code validation for `test\economic.refitter`; triage any post-merge generation failures separately -- Regression gates: existing merge-collision tests must continue passing +**Outcome:** APPROVED - ready for merge -- **2026-04-28T15:21:48.369+02:00 e-conomic multi-spec merge final validation:** Release build, focused `OpenApiDocumentFactoryMergeTests` (15 tests), focused `Issue1016_MultiSpecSchemaMergeTests` (7 tests), full Release solution tests, format verification, and the real `test\economic.refitter` CLI generation all passed. The required CLI command generated `test\GeneratedCode\economic.cs` successfully from `economic-products.json` + `economic-webhooks.json` without duplicate schema merge failures. +## 2026-05-29T15:07:47 - Scribe: Issue #1094 Commit Grouping Decision -## 2026-04-29: OpenApiDocumentFactory 100% Coverage & Obsolete Clone Validation +**Session:** commit-grouping-decision +**Context:** Finalized commit grouping and Squad documentation for issue #1094. -- **2026-04-28T19:19:33Z User Directive:** Get `src/Refitter.Core/OpenApiDocumentFactory.cs` to 100% coverage; it is one of the most important parts of the codebase and all possibilities should be tested thoroughly. -- **2026-04-29T08:41:29Z Decision:** Treat `NSwag.OpenApiDocument.Tags` and `NSwag.OpenApiDocument.Components.Schemas` as non-null collections inside `Merge()`, eliminating unreachable null-handling branches that blocked 100% coverage. -- **Validation Result:** Coverage command confirmed `OpenApiDocumentFactory` at 100% line/block coverage in `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage.cobertura.xml`. -- **Clone Obsolete API Validation:** Added focused `OpenApiDocumentFactoryMergeTests` covering Swagger 2/3 round-trip serialization, verifying parameterless `ToJson()` + `FromJsonAsync()` preserves `SchemaType` across versions and maintains merge-critical collections. -- **Outcome:** Obsolete `ToJson(SchemaType)` replacement confirmed safe by focused regression tests proving schema-type preservation and mixed-version merge correctness. +**Work:** +- Decision recorded: split product changes (tooling/CI, docs) and housekeeping (stale automation + Squad bookkeeping) into three separate commits +- Housekeeping commit intentionally excludes co-author trailer per Dallas decision +- Squad cross-agent updates and logs created +- Branch ready for merge after Squad documentation diff --git a/.squad/commit-msg.txt b/.squad/commit-msg.txt deleted file mode 100644 index 1d8eac18c..000000000 --- a/.squad/commit-msg.txt +++ /dev/null @@ -1,17 +0,0 @@ -Scribe: finalize breaking changes v2.0.0 guidance - -- Bishop: discussion draft + migration guide + TOC update -- Ripley: production-ready approval -- Scribe: orchestration logs + session log + cross-agent updates - -Artifacts: -- docs/docfx_project/articles/breaking-changes-v2-0-0.md (new) -- docs/docfx_project/articles/toc.yml (updated) -- .squad/orchestration-log/2026-04-18T10-46-51Z-bishop.md (new) -- .squad/orchestration-log/2026-04-18T10-46-51Z-ripley.md (new) -- .squad/log/2026-04-18T10-46-51Z-breaking-changes-guidance.md (new) -- .squad/agents/{bishop,ripley}/history.md (updated) - -Status: Ready for publication; discussion draft in session state pending user decision. - -Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> diff --git a/.squad/decisions-archive.md b/.squad/decisions-archive.md index 7fc9d87d1..37907e303 100644 --- a/.squad/decisions-archive.md +++ b/.squad/decisions-archive.md @@ -948,3 +948,597 @@ Recommendation: Address incrementally in 2.1.x patches. - The breaking-changes guide now documents the Microsoft.OpenApi/OasReader 1.x β†’ 3.x parser upgrade, expected generated-code diffs, and migration steps. - This removes the silent-upgrade surprise, but it is still not equivalent to corpus-based behavioral proof. + + +## Archived 2026-05-29 (entries older than 7 days) + +### 2026-05-01T14:04:19.681+02:00: Issue #1083 response framing +**By:** Bishop +**What:** Treat issue #1083 as a likely product bug rather than a documentation dispute; current docs scope identifier sanitization to contract properties under propertyNamingPolicy, not schema or contract type names. +**Why:** The maintainer reply should acknowledge the broken generated output without inaccurately claiming the docs already promised schema-type sanitization. + +### 2026-05-01T14:04:19.681+02:00: User directive +**By:** Christian Helle (via Copilot) +**What:** Use GPT-5.5 for all agents for the rest of this session only. +**Why:** User request β€” captured for team memory + +### 2026-05-01T14:34:56.630+02:00: Issue #1083 adjacent tooling verdict + +**By:** Dallas + +**Decision:** Do not add CLI, MSBuild, or README follow-up for #1083. The bug and its fix live in shared core type-name generation, so those surfaces inherit the corrected behavior without new wiring or user-facing settings. Add only source-generator regression coverage to prove the compile-time surface emits the sanitized DTO type and method return type for dotted schema names. + +**Why:** #1083 does not introduce a new option, command contract, settings shape, or consumer workflow. The adjacent risk was validation drift in the source-generator lane, so a focused generated-code test is the smallest correctness guard that keeps scope tight. + +# Lambert issue #1083 repro decision + +- **Date:** 2026-05-01T14:04:19.681+02:00 +- **Requester:** Christian Helle +- **Decision:** Treat GitHub issue #1083 as a valid current-HEAD bug and anchor the regression on a minimal inline Swagger 2 fixture that uses the exact dotted schema key `LookUpErnResponse.`. Keep any real Revenue-spec check as optional evidence only, not as the primary automated regression, to avoid live-network coupling. + +## Evidence + +- Minimal fixture generation produced `Task<> LookUpERN(...)` and `public partial class` with no identifier. +- Real Revenue spec generation produced the same failure shape in `IPAYEEnhancedReportingNotificationRESTAPIApi.cs` and `Contracts.cs`. +- Isolated compile checks failed with `CS1001 Identifier expected` for both generated outputs. + +## Required regression coverage + +1. Add a focused scenario test file in `src\Refitter.Tests\Scenarios` for invalid schema/type names. +2. Use an inline fixture with one endpoint whose 200-response references `#/definitions/LookUpErnResponse.`. +3. Assert generated code does **not** contain `Task<>` or `partial class` followed by a blank name. +4. Assert generated code **does** contain a concrete sanitized DTO identifier and that the response method returns that identifier. +5. Add a `BuildHelper.BuildCSharp(generatedCode).Should().BeTrue()` assertion so the regression proves compilable output, not just string replacement. +6. Only add/update `IdentifierUtilsTests` if the implementation explicitly routes schema/type-name sanitization through shared identifier utilities; otherwise keep coverage at the scenario level. + +## 2026-05-01T14:34:56.630+02:00 + +- Decision: keep issue #1083 coverage in a dedicated scenario test file instead of widening `PR1064BlockerRegressions`, `PropertyNamingPolicyTests`, or `IdentifierUtilsTests`. +- Why: the failure is schema/type-name generation, not property/parameter sanitization; isolating it behind a minimal inline fixture keeps the regression signal focused and avoids coupling unrelated suites to dotted-schema behavior. +- Required assertions: no blank `partial class`, no `Task<>`, `LookUpErnResponse` used consistently in generated contracts and interface signatures, plus a compile gate. + +--- +timestamp: 2026-05-01T14:34:56.630+02:00 +agent: parker +issue: 1083 +--- + +# Decision + +Implement issue #1083 at the schema type-name generation hook in `src\Refitter.Core\CSharpClientGeneratorFactory.cs`, not in downstream interface or contract post-processing. + +# Why + +- The failure starts in NSwag/NJsonSchema type resolution when a schema hint ends with an empty `.` segment. +- Repairing the hint before `DefaultTypeNameGenerator` runs fixes both DTO declarations and response signatures with one narrow change. +- To preserve normal behavior, malformed keys only normalize empty segments; if that normalized name collides with an existing clean schema key, the malformed schema is forced onto the counted suffix so the clean schema keeps the unsuffixed base name. + +# Validation + +- `dotnet format src\Refitter.slnx` +- `dotnet build -c Release src\Refitter.slnx` +- `dotnet test -c Release src\Refitter.slnx --no-build` (remaining failures were the known external-URL timeout lane in `PathParametersWithUrlTests`) +- `dotnet format --verify-no-changes src\Refitter.slnx` +- Manual CLI generation + scratch-project compilation for single-schema and collision fixtures + +# Parker Plan: Issue #1083 + +**Date:** 2026-05-01T14:04:19.681+02:00 +**Owner:** Parker +**Status:** Planning only + +## Decision + +Issue #1083 is a valid current bug in Refitter's generator pipeline. + + +- Reproduced at HEAD against `https://revenue-ie.github.io/paye-employers-documentation/PIT3/rest/paye-employers-rest-api-pit3.json`. +- Generated output contains both: + - `internal partial class` with no type name + - `Task<> LookUpERN(...)` +- The offending Swagger 2 definition key is `LookUpErnResponse.` and the `GET /ern/{employerRegistrationNumber}/{taxYear}` response references `#/definitions/LookUpErnResponse.` + +## Root Cause + +- Refitter currently relies on NSwag/NJsonSchema default type naming for schema names. +- NJsonSchema's `DefaultTypeNameGenerator` treats `.` as a segment separator and uses the last segment. +- For a trailing-dot name like `LookUpErnResponse.`, the last segment is empty, and the fallback path also returns an empty string instead of a usable anonymous name. +- That empty type name flows into both DTO emission and `generator.GetTypeName(...)`, producing the blank class declaration and empty generic return type. + +## Safest Fix Shape + +1. Add a Refitter-owned custom type-name generator that preserves current NSwag naming behavior for normal inputs. +2. Only special-case malformed hints whose final segment is empty or whose generated type name is blank. +3. Normalize those cases to the last non-empty segment (for example `LookUpErnResponse.` -> `LookUpErnResponse`) and still route the final identifier through Refitter-safe identifier sanitization. +4. Inject that generator in `CSharpClientGeneratorFactory` so the fix applies once at the DTO/type-resolution layer. + +## Main Files Likely Needed + +- `src\Refitter.Core\CSharpClientGeneratorFactory.cs` +- new core generator file such as `src\Refitter.Core\RefitterTypeNameGenerator.cs` +- possibly `src\Refitter.Core\IdentifierUtils.cs` if a shared helper is introduced for final identifier normalization +- regression coverage in `src\Refitter.Tests\Scenarios\PR1064BlockerRegressions.cs` or a dedicated scenario test file +- possibly `src\Refitter.Tests\IdentifierUtilsTests.cs` if helper behavior moves into `IdentifierUtils` + +## Risks / Tradeoffs + +- Any type-name generator change can rename emitted contracts for malformed schema names, so tests should pin current behavior for ordinary names. +- Avoid broad dot replacement: dotted names may already rely on NSwag's "use last segment" behavior, so the fix should target only empty-tail cases. +- Collisions remain possible if multiple malformed names normalize to the same identifier; rely on NSwag's reserved-name flow or add focused collision coverage. +- Rewriting schema keys and refs in the OpenAPI document would be much riskier because it can disturb references, exclusions, and other preprocessing logic. + + +# Squad Decisions + +## 2026-04-21 + +### PR #1067 Linked-Issue Closure Matrix + +**Lead:** Ripley +**Status:** REVIEWED + +- Treat **#1017, #1022, #1023, #1024, and #1026** as fully closed on the reviewed branch state. +- Treat **#1025** as **partial/documentation-first only**; do not auto-close it from PR wording. +- Final review guidance requires removing or downgrading `Fixes #1025` in the PR body so GitHub does not overstate closure. + +### Documentation and Package Guidance Alignment + +**Verified By:** Bishop / Ash +**Status:** REQUIRED AND VERIFIED + +- `Refitter.SourceGenerator` package guidance must describe Roslyn `AddSource()` behavior rather than legacy disk-file output. +- Consumer guidance must explicitly require a direct `Refit` reference (and `Refit.HttpClientFactory` when generated DI helpers are used). +- Disk-output settings (`outputFolder`, `contractsOutputFolder`, `generateMultipleFiles`) should be documented as CLI/MSBuild-oriented, not source-generator disk artifacts. +- Final safety-lane review approved PR #1067 once issue-closure wording was honest and the packaging/docs/test evidence aligned. + +### Session Directives Archived + +**By:** Christian Helle (via Copilot) + +- 2026-04-20: Commit changes as often as possible in small logical groups. +- 2026-04-21: Use Opus for all agents for the rest of that session only. +- 2026-04-21: Commit changes in small logical groups. +- 2026-04-25: Use GPT-5.5 for all agents for the rest of this session only. + +## 2026-04-25 + +### Remaining Audit Matrix Pass (#1057) + +**Verified By:** Ripley +**Status:** VERIFIED + +- Treat **#1042** as **validation-only** until a concrete Spectre.Console.Cli parsing regression is reproduced at current HEAD. +- Treat **#1047** as **already fixed / stale issue text** at current HEAD because MSBuild now consumes CLI-emitted `GeneratedFile:` markers instead of regex-parsing `.refitter` JSON for output paths. +- Treat **#1056** as **doc/invariant-only** for now; preserve the current generation ordering/state flow and document the invariant before changing behavior. +- Treat **#1032** as **validation-first**; gather runtime evidence before changing enum-converter behavior. +- Coordination note from the verification pass: **#1045 and #1047 appear already fixed at HEAD**, and the remaining code-backed follow-up stays with Dallas/Parker. + +### Remaining Audit Repro Pass (#1057) + +**Verified By:** Lambert (Tester) +**Status:** EVIDENCE NARROWED + +- Treat **#1028** as **still reproducible by inspection** on current HEAD; the source-generator incremental pipeline still carries a `List` equality hazard. +- Treat **#1029** as **partial** on current HEAD; visible diagnostics improved, but the "no .refitter files found" path is still only `Debug.WriteLine`. +- Treat **#1033** as **still reproducible by inspection**; enum-converter injection still uses a hard-coded LF and needs newline normalization coverage. +- Treat **#1041** as **partial**; runtime resolution improved, but argument escaping and timeout kill semantics still leave repro surface. +- Treat **#1043** as **still reproducible**; legacy `--generate-authentication-header` bool-style CLI usage still fails at current HEAD. +- Treat **#1032, #1042, #1045, and #1047** as **validation-only / fixed-at-HEAD evidence** unless stronger failing repros appear. +- Treat **#1034, #1039, and #1056** as **not reproduced on current HEAD** in Lambert's pass. + +### Multi-spec Merge Collision Policy + +**Decided By:** Parker (Core Developer) +**Status:** APPROVED + +- `OpenApiDocumentFactory` should clone the first loaded document before merging additional specs so callers do not observe mutation of a previously loaded `OpenApiDocument`. +- Path and schema-key collisions across distinct OpenAPI inputs should fail fast with `InvalidOperationException` instead of silently keeping the first definition. +- Exact duplicate input paths should continue to deduplicate up front so feeding the same spec twice stays harmless. + +**Rationale:** Silent first-one-wins merge behavior hides real API-shape conflicts and only surfaces later during generation or runtime use. Failing fast is the safer core-library policy. + +### Core Lane Follow-up Gate (#1057) + +**Verified By:** Parker (Core Developer) +**Status:** FIXED / NARROWED + +- **#1033**: landed at HEAD with a code change in `src/Refitter.Core/RefitGenerator.cs` plus regression coverage in `src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs`. +- **#1032**: treat as validation-first pending review gate; no current core-lane code change required. +- **#1034** and **#1039**: treat as fixed-at-HEAD / no-repro on the reviewed branch state pending final gate review. +- **#1045**: treat as fixed-at-HEAD on the reviewed branch state pending final gate review. +- **#1056**: treat as doc/invariant-only for now; preserve current ordering behavior unless new failing evidence appears. +### Tooling Compatibility Follow-up + +**Verified By:** Dallas (Tooling Developer) +**Status:** APPROVED + +- Preserve CLI compatibility for `--generate-authentication-header` by treating the legacy boolean forms (`true`, `false`) and the bare flag as valid inputs. The bare flag and `true` now map to `AuthenticationHeaderStyle.Method`; `false` maps to `None`, while `Parameter` still requires the explicit enum value. +- Keep MSBuild runtime resolution resilient across both packed and test-project layouts. The task now prefers bundled framework-specific Refitter binaries, falls back to lower compatible TFMs when probing fails, and finally uses a co-located `refitter.dll` when the packaged layout is unavailable. + +### PR Prep Closure Guidance + +**Verified By:** Ripley +**Status:** DRAFTED FOR PR ASSEMBLY + +- Safe auto-close candidates on the reviewed branch state: **#1028**, **#1029**, **#1033**, and **#1043**. +- Keep **#1032**, **#1034**, **#1039**, **#1041**, **#1042**, **#1045**, **#1047**, and **#1056** out of PR auto-close wording until stronger evidence or final lane approval exists. +- **#1041** specifically remains a Dallas-owned tooling verdict before any PR body claims closure. + +### Ash core review of remaining #1057 closures + +**Verified By:** Ash +**Status:** REJECT + +- Verified acceptable: + - **#1032** does not reproduce the claimed custom `JsonNamingPolicy` override regression at current HEAD; runtime repro with a type-level `JsonStringEnumConverter` still serialized via `JsonSerializerOptions.Converters` (`"my_value"`). + - **#1045** is effectively fixed at current HEAD because `RefitGenerator.GetOpenApiDocument()` uses `OpenApiPaths` directly when populated instead of dereferencing `OpenApiPath`. + - **#1033** is the only intentional core code change in the working tree (`src/Refitter.Core/RefitGenerator.cs`) and it has matching regression coverage in `src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs`. +- Still open / false closure: + - **#1034** remains open in `src/Refitter.Core/OpenApiDocumentFactory.cs:55-107`; `Merge()` still mutates `documents[0]` and still silently keeps the first path/schema on key collisions. + - **#1039** remains open in `src/Refitter.Core/ParameterExtractor.cs:447-487` plus `src/Refitter.Core/RefitInterfaceGenerator.cs:69-82`; `GetParameters()` removes query parameters from `operationModel.Parameters` before XML-doc generation reads the shared model. +- Follow-up requirement: reassign the remaining core revisions to Parker (or another core implementer) for real fixes before closing **#1034**/**#1039** from the `#1057` matrix. + +### Dallas core revision on rejected blockers + +**Verified By:** Dallas +**Status:** IMPLEMENTED / PENDING ASH RE-REVIEW + +- **#1034:** `OpenApiDocumentFactory.Merge()` now clones the first input before merge so callers no longer observe mutation of a previously loaded `OpenApiDocument`. +- **#1034:** duplicate path/schema collisions now emit warnings while preserving the existing merged entry; this revised pass does **not** follow the earlier fail-fast proposal. +- **#1039:** `ParameterExtractor` no longer mutates the shared `operationModel.Parameters` collection when building grouped query-parameter wrappers, so downstream consumers keep the original operation model intact. +- Regression coverage was refreshed for the revised core pass in `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs`, `src/Refitter.Tests/ParameterExtractorEdgeCaseTests.cs`, `src/Refitter.Tests/ParameterExtractorPrivateCoverageTests.cs`, and `src/Refitter.Tests/RegressionTests/Issue1039_DynamicQuerystringMutationTests.cs`. +- Dallas reported the revised core validation lane green; Ash is performing re-review and Lambert is reconciling the blocker-test lane against the landed behavior. + +### Ash core re-review of Dallas revision + +**Verified By:** Ash +**Status:** PARTIAL / BLOCKED + +- **#1039 resolved:** ParameterExtractor.GetParameters() no longer mutates operationModel.Parameters, and ParameterExtractorPrivateCoverageTests now lock that invariant for XML-doc generation and shared-model reuse. +- **#1034 still open:** OpenApiDocumentFactory.Merge() now clones the first input, but it still keeps the first conflicting path/schema/definition/security entry via Trace.TraceWarning(...) instead of failing fast. +- src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs still codifies warning-backed first-wins collision handling; the next narrow revision must flip that coverage to an InvalidOperationException contract for conflicting inputs. +- Do **not** close **#1034** from the #1057 matrix yet. Dallas owns one last narrow revision, and Lambert remains on the blocker-test lane. + +### Dallas final #1034 revision / Lambert blocker-test reconciliation + +**Verified By:** Dallas / Lambert +**Status:** IMPLEMENTED / READY FOR ASH FINAL GATE + +- `OpenApiDocumentFactory.Merge()` now preserves the clone-first non-mutation guarantee **and** fails fast with `InvalidOperationException` when distinct inputs introduce conflicting duplicate path, schema, definition, or security keys. +- Non-conflicting merges still return a new document without mutating either input document, and exact duplicate input paths remain harmless because they are deduplicated before merge. +- Blocker coverage is now aligned to the fail-fast contract in `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs`; `Issue1039_DynamicQuerystringMutationTests.cs` still preserves grouped-query XML-doc assertions across single-interface, `MultipleInterfaces.ByTag`, and `MultipleInterfaces.ByEndpoint` generation. +- Dallas's final narrow implementation pass is complete. Ash is performing the final review gate, and Lambert is reconciling the blocker-test lane against the landed fail-fast behavior. + +### Ash final core gate rejection + +**Verified By:** Ash +**Status:** REJECTED + +- **#1034** is still not proven closed: `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs` only demonstrates fail-fast behavior for duplicate **paths**, not explicit conflicting **schemas**, **definitions**, and **security schemes**. +- The broader core validation lane is not green because `dotnet test -c Release src\Refitter.Tests\Refitter.Tests.csproj` still fails `Dynamic_Querystring_Generation_Preserves_Original_Query_Param_Documentation(ByEndpoint)` in `Issue1039_DynamicQuerystringMutationTests`. +- Dallas is now locked out of the next revision cycle for this artifact; Parker remains locked out from the prior rejected cycle. +- Lambert now owns the next/final revision cycle for **#1034** while staying in the blocker-test lane. + +### Ash final review of Lambert revision + +**Verified By:** Ash +**Status:** REJECTED + +- **#1039 acceptable:** `ParameterExtractor.GetQueryParameters()` still snapshots query parameters locally and preserves the shared `operationModel.Parameters` list; the regression coverage remains aligned with the intended non-mutating behavior. +- **#1034 still not proven closed:** the Swagger 2 definition-collision proof is still not isolated cleanly enough. `OpenApiDocumentFactoryMergeTests.Merge_With_Definition_Collision_Throws_And_Does_Not_Mutate_Inputs` still trips the duplicate **schema** conflict before it conclusively proves the duplicate **definition** lane. +- Remaining blocker for the next cycle: isolate the definition-specific fail-fast proof so the test fails for the intended definition-collision reason instead of the mirrored schema path. +- Lambert now joins Parker and Dallas in lockout for the next revision cycle on this artifact. +- Ripley now owns the next narrow revision cycle for **#1034**. + +### Ripley final #1034 proof-gap revision + +**Verified By:** Ripley +**Status:** IMPLEMENTED / PENDING ASH FINAL SIGNOFF + +- Preserve the source document schema type during clone/copy so Swagger 2 inputs stay on the intended definitions surface throughout merge handling. +- Isolate the Swagger 2 definition-collision proof at MergeIfMissingOrThrowOnConflict(...) so the definition-specific fail-fast contract is asserted directly instead of being masked by the mirrored schema collision first. +- Reported validation from the revision lane is green for dotnet build -c Release src\Refitter.slnx and dotnet test -c Release src\Refitter.Tests\Refitter.Tests.csproj. +- Ash now owns the final reviewer signoff before broader validation resumes. +### Ash final signoff on Ripley #1034/#1039 follow-up + +**Verified By:** Ash +**Status:** APPROVED + +- **#1034 approved:** OpenApiDocumentFactory.Merge() now clones the first document before merge, fails fast on conflicting duplicate path/schema/definition/security keys, and isolates the remaining Swagger 2 definition proof through the shared MergeIfMissingOrThrowOnConflict(...) path. +- **#1039 approved:** grouped dynamic-query extraction still snapshots query parameters instead of mutating operationModel.Parameters, and XML-doc regression coverage remains locked for single-interface, ByTag, and ByEndpoint generation. +- Evidence reviewed: src/Refitter.Core/OpenApiDocumentFactory.cs, src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs, src/Refitter.Tests/RegressionTests/Issue1039_DynamicQuerystringMutationTests.cs, and src/Refitter.Tests/ParameterExtractorPrivateCoverageTests.cs. +- Reviewer signoff was reported against dotnet test -c Release src\Refitter.Tests\Refitter.Tests.csproj with 1840 passing and 0 failing. + +### Final PR package guidance for #1057 + +**Prepared By:** Ripley +**Status:** READY FOR PR ASSEMBLY + +- Proposed PR title: `[v2.0 audit] Close remaining verified #1057 regressions`. +- Keep PR summary focused on five landed lanes: source-generator diagnostics, newline-safe enum-converter rewriting, non-mutating dynamic querystring generation, fail-fast multi-spec merge handling, and tooling/runtime compatibility hardening. +- Safe auto-close set for the final PR body: **#1028, #1029, #1033, #1034, #1039, #1041, #1043**. +- Keep **#1032, #1042, #1045, #1047, and #1056** out of auto-close wording because they are validation-only, fixed-at-HEAD/stale, or doc/invariant-only. +- Before opening the PR, recreate/publish v2.0.0-prerelease-fixes with `git push -u origin HEAD` because the local branch tracks a gone upstream. +- Latest local full validation reported: dotnet restore src\Refitter.slnx, dotnet build -c Release src\Refitter.slnx --no-restore, dotnet test -c Release src\Refitter.slnx --no-build, and dotnet format --verify-no-changes src\Refitter.slnx --no-restore with 1886 tests passing. + +### CLI help output assertions should be semantic + +**Verified By:** Lambert (Tester) +**Status:** APPROVED + +- src\Refitter\Program.cs intentionally rewrites a no-argument invocation to --help, exits 0, and emits Spectre.Console.Cli help output. +- The current product behavior is correct; the instability sits in whitespace-sensitive test expectations, not in production code. +- src\Refitter.Tests\GenerateCommandTests.cs should assert semantic help markers (usage pattern, sections, and known option names) instead of exact formatter-driven spacing/default-value layout. +- Validation reported: release run of src\Refitter.Tests\Refitter.Tests.csproj, focused rerun of Program_Main_Should_Show_Help_When_Invoked_Without_Arguments, and dotnet format --verify-no-changes src\Refitter.slnx. +- Cross-agent merge outcome: Dallas proved the Ubuntu failure was ANSI/wrapping noise in raw Spectre.Console help output, and Lambert landed the test-only fix in src\Refitter.Tests\GenerateCommandTests.cs by normalizing redirected console output before asserting semantic help markers. +- Final landed validation for the product commit `normalize help output test across platforms`: dotnet build -c Release src\Refitter.slnx, dotnet test -c Release src\Refitter.slnx, and dotnet format --verify-no-changes src\Refitter.slnx. + +### RefitterGenerateTask edge-case coverage stays test-only + +**Verified By:** Dallas / Lambert +**Status:** APPROVED + +- Preserve the current `src\Refitter.MSBuild\RefitterGenerateTask.cs` behavior and close the remaining coverage gap with regression tests instead of production changes. +- Lambert isolated the last uncovered branches to `TryExecuteRefitter()` exception handling, missing bundled CLI handling in `StartProcess()`, `ResolveRefitterDll()` fallback edges, sub-second timeout formatting, and the non-throwing `TryLogErrorFromException()` path. +- Dallas landed the test-only closure in `src\Refitter.Tests\RefitterGenerateTaskTests.cs`, covering blank package folders, whitespace runtime entries, co-located and first-bundled fallback resolution, missing bundled CLI failure, process-runner exception handling, millisecond timeout formatting, and successful `LogErrorFromException` forwarding. +- Reported validation: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`, `dotnet build -c Release src\Refitter.slnx --no-restore`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. +- Result: `RefitterGenerateTask.cs` reached 100% line coverage, 100% block coverage, and 0 partial functions in the reported coverage output. +### PR #1070 SonarCloud quality-gate repair + +**Verified By:** Dallas / Ash / Parker +**Status:** APPROVED + +- Keep the ParameterExtractor (S1066) and RefitterGenerateTask (S3267, S3358) changes narrow and behavior-preserving; Ash approved those cleanups as safe. +- Keep the source-generator diagnostic cleanup on the stable one-descriptor-per-ID contract: reuse the shared Refitter title/category constant, but assign distinct IDs when the title/message semantics differ. +- Preserve GeneratedDiagnostic as a readonly record struct; retain the explicit ordinal GetHashCode() behavior and handle Sonar S1206 with a targeted suppression plus justification instead of rewriting the type into a manual struct. +- Final validation recorded for the landed artifact: dotnet build -c Release src\Refitter.slnx --no-restore, dotnet test -c Release --solution src\Refitter.slnx --no-build, and dotnet format --verify-no-changes src\Refitter.slnx --no-restore. + +## 2026-04-26 + +### AI-slop cleanup sequencing + +**Verified By:** Ripley +**Status:** APPROVED + +- Start with **docs/help drift** so stale SourceGenerator and MSBuild guidance is corrected before code refactors. +- Follow with **settings/spec-path normalization** across CLI and source-generator entry points, then centralize the shared **`GeneratedFile:` marker contract**. +- Keep **test-surface cleanup** ahead of deeper generator dedup so regression coverage is stronger before touching contract-sensitive code. +- Leave **core generator dedup** for the last batch and treat `OpenApiDocumentFactory` merge semantics, auth-header CLI compatibility, source-generator diagnostic contracts, and single-file `GeneratedCode` metadata behavior as reviewer-gated cleanup seams. +- Current squad capacity is sufficient for docs, tests, tooling, and settings cleanup; bring in a specialized C# reviewer only for the later core generator / `ParameterExtractor` dedup lane. + +### Cleanup-safe baseline and coverage guardrails + +**Verified By:** Lambert (Tester) +**Status:** VERIFIED + +- Baseline validation is healthy on this machine: restore, release build, solution tests, and format verification all passed. +- Treat the Codecov command in `.github\workflows\codecov.yml` as the authoritative cleanup coverage lane; it targets `src\Refitter.Tests\Refitter.Tests.csproj`. +- Prefer narrow branch-coverage gains before production edits in `src\Refitter.Core\ContractTypeSuffixApplier.cs`, `SchemaCleaner.cs`, `CSharpClientGeneratorFactory.cs`, `XmlDocumentationGenerator.cs`, `IdentifierUtils.cs`, and `src\Refitter\Settings.cs`. +- Do not use the live-network test surface (`OpenApiDocumentFactoryTests`, `SwaggerPetstoreTests`, `SwaggerPetstoreApizrTests`, `Examples\OpenApiUrlTests`) as a stability signal during cleanup because it remains environment-sensitive. +- Coverage reports also include `.test-work` runtime-proof artifacts; note them, but prioritize real repository source files first. + +### Generator cleanup phasing + +**Verified By:** Parker (Core Developer) +**Status:** APPROVED + +- Land generator cleanups in two phases: first trim redundant string/branch duplication already covered by public regressions in `XmlDocumentationGenerator.AppendXmlCommentBlock()` and `ContractTypeSuffixApplier.TypeSuffixRewriter`. +- Before deeper dedupe, add or tighten compile-backed public regressions for `ParameterExtractor`, multipart generation, and interface-emission behavior. +- Route any cleanup that deduplicates `RefitGenerator.Generate()` vs `GenerateMultipleFiles()` or the shared method-emission logic across the three interface generators through **Ash review**. + +### AI-slop safety review gate + +**Verified By:** Ash +**Status:** REQUIRED + +- Treat the safety-sensitive generator and MSBuild cleanup lane as **validation-first** rather than style-first. +- Require source-generator regressions proving same-directory `.refitter` files with the same `outputFilename` do not collide on `AddSource()` hint names and that `AdditionalText.GetText(...) == null` reports a diagnostic instead of throwing. +- Require generator regressions for OpenAPI-title-derived identifiers containing `<` / `>` and for ByEndpoint interface names whose internal `I` characters must remain intact. +- Require MSBuild coverage for `GetInstalledDotnetRuntimes()` timeout/failure handling before claiming the process-launch path is hardened. + +### Session directives refresh + +**By:** Christian Helle (via Copilot) + +- 2026-04-26: Have all agents use GPT-5.5 for the rest of this session only. +- 2026-04-26: Commit changes as frequent as possible in small logical groups for a detailed progress history. + +## 2026-04-28 + +### e-conomic Multi-Spec OpenAPI Merge Failure + +**Leads:** Ripley (analysis), Parker (core finding), Dallas (tooling validation) +**Tester:** Lambert +**Status:** TRIAGED AND APPROVED FOR FIX + +#### Findings + +- `test\economic.refitter` triggers merge failure in `OpenApiDocumentFactory.Merge()` while combining `economic-products.json` and `economic-webhooks.json`. +- Exception: `InvalidOperationException: Cannot merge OpenAPI documents because a duplicate schema 'Error' was found.` +- Root cause: `AreEquivalent()` calls `Serializer.Serialize()` on NSwag/NJsonSchema objects; object-cycle behavior returns `false` for semantically identical schemas. +- Both `Error` and `ProblemDetails` are duplicated and textually equivalent in source JSON; merge fails at `Error` first. +- Issue is **not** in `.refitter` parsing, path resolution, or individual spec generationβ€”each spec generates successfully in isolation. +- CLI, source generator, and MSBuild all reach the same merge failure point before code output. + +#### Fix Ownership and Scope + +**Primary owner:** `Refitter.Core` β†’ `src\Refitter.Core\OpenApiDocumentFactory.cs` β†’ `MergeIfMissingOrThrowOnConflict()` / `AreEquivalent()` + +- Replace `AreEquivalent()` with OpenAPI-aware semantic equivalence check (use NSwag `ToJson()` or canonical JSON representation). +- Keep fail-fast policy for **genuinely conflicting** duplicate path/schema/definition/security entries. +- Keep clone-first/non-mutating merge behavior. +- Do **not** edit e-conomic OpenAPI specs; do **not** rename schemas in merge. + +#### Approved Test Coverage and Gates + +1. Add `OpenApiDocumentFactoryMergeTests`: + - `Merge_With_Equivalent_Duplicate_Schema_Does_Not_Throw` β€” OpenAPI 3 docs with identical schema key and body; both paths preserved. + - Preserve existing `Merge_With_Schema_Collision_Throws_And_Does_Not_Mutate_Inputs` for conflicts. + - Optional e-conomic-shaped regression for `Error` schema with descriptions, nullable properties, extensions, `additionalProperties: false`. + +2. Compile-backed regression: two specs with identical common error schema and distinct paths; assert generation succeeds and generated code builds. + +3. Run generated-code validation for `test\economic.refitter` after merge fix; triage any post-merge generation failure separately. + +4. Existing merge-collision tests must continue passing. + +#### Constraints + +- Preserve relative-path behavior across CLI and source generator. +- Keep `openApiPaths` semantics consistent. +- OpenAPI 3 `components.schemas` and Swagger 2 `definitions` equivalence must remain aligned. +- Do not hide genuine schema differences in comparison. + +### Ash economic merge review + +- Date: 2026-04-28T15:21:48.369+02:00 + +Verdict: APPROVED + +Rationale: +- `OpenApiDocumentFactory.Merge()` still fails fast on genuinely conflicting duplicate paths, schemas, Swagger 2 definitions, and security schemes while accepting equivalent duplicate schemas. +- Equivalence is based on normalized OpenAPI/NSwag/NJsonSchema JSON tokens rather than object identity; the recursive schema test covers cyclic schema graphs and the e-conomic regression verifies real multi-spec generation remains buildable. +- Focused validation passed for `OpenApiDocumentFactoryMergeTests`, `Issue1016_MultiSpecSchemaMergeTests`, and the duplicate-path merge coverage via the net10.0 TUnit executable. A first `dotnet test --filter` attempt failed because this TUnit project does not support that option, not because of product/test failure. + +### Lambert e-conomic validation + +- Date: 2026-04-28T15:21:48.369+02:00 +- Validation result: PASS +- Evidence: + - `dotnet build -c Release src\Refitter.slnx` passed. + - Focused `OpenApiDocumentFactoryMergeTests` passed: 15 succeeded / 0 failed. + - Focused `Issue1016_MultiSpecSchemaMergeTests` passed: 7 succeeded / 0 failed. + - `dotnet test -c Release src\Refitter.slnx` passed. + - `dotnet format --verify-no-changes src\Refitter.slnx` passed. + - `dotnet .\src\Refitter\bin\Release\net9.0\refitter.dll --settings-file test\economic.refitter --no-banner --no-logging --simple-output` passed and generated `test\GeneratedCode\economic.cs`. +- Decision signal: the e-conomic duplicate equivalent schema merge path is validated by focused regression tests, full solution tests, and real CLI generation. + +### Parker: e-conomic equivalence implementation + +- Date: 2026-04-28T15:21:48.369+02:00 +- Decision: `OpenApiDocumentFactory.AreEquivalent()` should compare normalized OpenAPI/NSwag JSON first and use a narrow NJsonSchema structural fallback only for schemas whose standalone JSON serialization cannot resolve references. +- Rationale: duplicate schemas from separate vendor specs can be semantically identical while NJsonSchema object graphs contain unresolved/cyclic references; raw serializer comparison falsely reports conflicts. +- Guardrail: merge still copies missing keys, ignores equivalent duplicates, and throws `InvalidOperationException` for non-equivalent duplicate paths, schemas, definitions, and security schemes. +- Validation: focused merge tests and Issue1016 multi-spec regression tests passed. + +### User directive: 100% coverage on OpenApiDocumentFactory + +**By:** Christian Helle (via Copilot) +**Date:** 2026-04-28T19:19:33.388+02:00 + +Get `src/Refitter.Core/OpenApiDocumentFactory.cs` to 100% coverage; it is one of the most important parts of the code base and all possibilities should be tested thoroughly. + +### Ripley: GitHub Issue/PR Prep for e-conomic Multi-Spec Fix + +**Author:** Ripley (Lead) +**Date:** 2026-04-28 +**Status:** READY FOR EXECUTION ON WORDING + +#### Decision + +For this repo, the pending issue + PR sequence should target **`main`**, not `dev`, because the live remote has no `dev` branch and GitHub reports `main` as the default branch. + +The current execution branch is **`economic-openapi`**. It is already pushed, tracks `origin/economic-openapi`, has no open PR, and carries the seven fix commits for the e-conomic multi-spec merge work. + +`test/multiple-sources.refitter` does **not** currently block PR creation. It is already committed on the branch and the working tree is clean, so no extra stash/reset step is required before creating the issue and PR. + +#### Why + +- Repo reality overrides the generic dev-first squad workflow here. +- Using `main` avoids opening a PR against a non-existent branch. +- A clean working tree means the PR can safely describe only the already-committed fix set. + +#### Execution Notes + +1. Create the GitHub issue once Bishop provides final wording. +2. Re-push `economic-openapi` only if a last-minute commit is added after wording arrives. +3. Open the PR from `economic-openapi` to `main` and include `Closes #` in the PR body. + +### Lambert: OpenApiDocumentFactory 100% Coverage + +**Owner:** Lambert +**Date:** 2026-04-28 + +#### Decision + +Treat `NSwag.OpenApiDocument.Tags` and `NSwag.OpenApiDocument.Components.Schemas` as non-null collections inside `OpenApiDocumentFactory.Merge`, and use collection-count checks instead of null-coalescing/null-conditional branches there. + +#### Why + +Local reflection and coverage evidence showed the previous null-handling branches in `Merge` were not reachable for parsed NSwag documents because: + +- `Tags` materializes as an empty list, not `null` +- `Components` materializes as a non-null object +- `Components.Schemas` materializes as an empty dictionary, not `null` + +Those unreachable branches blocked `OpenApiDocumentFactory.cs` from reaching 100% coverage even after adding focused regression tests. + +#### Evidence + +- Full coverage command passed and reported `OpenApiDocumentFactory` functions at 100% line/block coverage in `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage.cobertura.xml` +- Reflection check confirmed NSwag parses both OpenAPI 3 and Swagger 2 documents with non-null `Tags`, `Components`, and `Components.Schemas` + +#### Impact + +- No intended behavior change for real parsed OpenAPI inputs +- Merge logic is simpler and now fully coverable with focused regression tests + +## 2026-04-29 + +### Ash: OpenApiDocument clone path should use parameterless ToJson() + +**Author:** Ash +**Date:** 2026-04-29T10:41:29.997+02:00 + +#### What + +For `src\Refitter.Core\OpenApiDocumentFactory.cs`, replace the obsolete `OpenApiDocument.ToJson(SchemaType)` clone path with `OpenApiDocument.ToJson()` followed by `OpenApiDocument.FromJsonAsync(...)`. + +#### Why + +Focused coverage in `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs` shows the non-obsolete serializer preserves `SchemaType` and the merge-critical collections for embedded Swagger 2 and OpenAPI 3 fixtures, while mixed-version merge assertions confirm the cloned base document still serializes with the correct top-level version marker. That keeps the clone-first/fail-fast merge contract intact without carrying the obsolete API forward. + +### Lambert: OpenApi clone validation surface + +**Author:** Lambert +**Date:** 2026-04-29T10:41:29.997+02:00 + +#### What + +Validate the obsolete `OpenApiDocument.ToJson(SchemaType)` replacement through focused `OpenApiDocumentFactoryMergeTests` that round-trip embedded Swagger Petstore JSON fixtures for both Swagger 2 and OpenAPI 3, then assert merge keeps the base document schema type. + +#### Why + +The obsolete call only exists to clone the first `NSwag.OpenApiDocument` before merge. The safest proof is not a full multi-spec integration run first; it is a narrow clone/merge surface that confirms the cloned document keeps `SchemaType` and the key collections (`Paths`, `Tags`, `Components.Schemas`, `Definitions`, `SecurityDefinitions`) that merge relies on. + +### User directive: Use GPT-5.5 for agents + +**By:** Christian Helle (via Copilot) +**Date:** 2026-04-29T10:41:29.997+02:00 + +Use GPT-5.5 for all agents for the rest of this session only. + +### Ripley PR Assembly (PR #1079) + +**Author:** Ripley (Lead) +**Date:** 2026-04-29T11:51:36.530+02:00 +**Branch:** `build-warnings` +**PR:** #1079 `Tighten warning handling across Refitter builds` +**Status:** READY FOR REVIEW + +#### Decision + +Frame the PR around the branch's real aggregate scope instead of only the newest OpenAPI clone-warning fix. + +#### Rationale + +- The branch includes multiple warning-focused commits across core, tests, CLI packaging metadata, nullable-flow cleanup, and warning enforcement changes. +- It already carries a committed `.squad` decision/history sync. +- The obsolete `OpenApiDocument` clone fix remains important and should stay called out explicitly, but only as one bullet within the broader warning-hardening frame. + +#### Reviewer Guidance + +- Review against `main`. +- Expect main product deltas in `src\Refitter.Core\OpenApiDocumentFactory.cs`, `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs`, `src\Refitter.Core\Refitter.Core.csproj`, `src\Refitter.Tests\Refitter.Tests.csproj`, and `src\Refitter\Refitter.csproj`. +- Treat the `.squad` history/decision sync as a carried branch artifact, not the headline behavior change. + diff --git a/.squad/decisions.md b/.squad/decisions.md index 1cde2dd83..97c714545 100644 --- a/.squad/decisions.md +++ b/.squad/decisions.md @@ -1,589 +1,52 @@ -### 2026-05-01T14:04:19.681+02:00: Issue #1083 response framing -**By:** Bishop -**What:** Treat issue #1083 as a likely product bug rather than a documentation dispute; current docs scope identifier sanitization to contract properties under propertyNamingPolicy, not schema or contract type names. -**Why:** The maintainer reply should acknowledge the broken generated output without inaccurately claiming the docs already promised schema-type sanitization. +## 2026-05-29 -### 2026-05-01T14:04:19.681+02:00: User directive -**By:** Christian Helle (via Copilot) -**What:** Use GPT-5.5 for all agents for the rest of this session only. -**Why:** User request β€” captured for team memory - -### 2026-05-01T14:34:56.630+02:00: Issue #1083 adjacent tooling verdict - -**By:** Dallas - -**Decision:** Do not add CLI, MSBuild, or README follow-up for #1083. The bug and its fix live in shared core type-name generation, so those surfaces inherit the corrected behavior without new wiring or user-facing settings. Add only source-generator regression coverage to prove the compile-time surface emits the sanitized DTO type and method return type for dotted schema names. - -**Why:** #1083 does not introduce a new option, command contract, settings shape, or consumer workflow. The adjacent risk was validation drift in the source-generator lane, so a focused generated-code test is the smallest correctness guard that keeps scope tight. - -# Lambert issue #1083 repro decision - -- **Date:** 2026-05-01T14:04:19.681+02:00 -- **Requester:** Christian Helle -- **Decision:** Treat GitHub issue #1083 as a valid current-HEAD bug and anchor the regression on a minimal inline Swagger 2 fixture that uses the exact dotted schema key `LookUpErnResponse.`. Keep any real Revenue-spec check as optional evidence only, not as the primary automated regression, to avoid live-network coupling. - -## Evidence - -- Minimal fixture generation produced `Task<> LookUpERN(...)` and `public partial class` with no identifier. -- Real Revenue spec generation produced the same failure shape in `IPAYEEnhancedReportingNotificationRESTAPIApi.cs` and `Contracts.cs`. -- Isolated compile checks failed with `CS1001 Identifier expected` for both generated outputs. - -## Required regression coverage - -1. Add a focused scenario test file in `src\Refitter.Tests\Scenarios` for invalid schema/type names. -2. Use an inline fixture with one endpoint whose 200-response references `#/definitions/LookUpErnResponse.`. -3. Assert generated code does **not** contain `Task<>` or `partial class` followed by a blank name. -4. Assert generated code **does** contain a concrete sanitized DTO identifier and that the response method returns that identifier. -5. Add a `BuildHelper.BuildCSharp(generatedCode).Should().BeTrue()` assertion so the regression proves compilable output, not just string replacement. -6. Only add/update `IdentifierUtilsTests` if the implementation explicitly routes schema/type-name sanitization through shared identifier utilities; otherwise keep coverage at the scenario level. - -## 2026-05-01T14:34:56.630+02:00 - -- Decision: keep issue #1083 coverage in a dedicated scenario test file instead of widening `PR1064BlockerRegressions`, `PropertyNamingPolicyTests`, or `IdentifierUtilsTests`. -- Why: the failure is schema/type-name generation, not property/parameter sanitization; isolating it behind a minimal inline fixture keeps the regression signal focused and avoids coupling unrelated suites to dotted-schema behavior. -- Required assertions: no blank `partial class`, no `Task<>`, `LookUpErnResponse` used consistently in generated contracts and interface signatures, plus a compile gate. - ---- -timestamp: 2026-05-01T14:34:56.630+02:00 -agent: parker -issue: 1083 ---- - -# Decision - -Implement issue #1083 at the schema type-name generation hook in `src\Refitter.Core\CSharpClientGeneratorFactory.cs`, not in downstream interface or contract post-processing. - -# Why - -- The failure starts in NSwag/NJsonSchema type resolution when a schema hint ends with an empty `.` segment. -- Repairing the hint before `DefaultTypeNameGenerator` runs fixes both DTO declarations and response signatures with one narrow change. -- To preserve normal behavior, malformed keys only normalize empty segments; if that normalized name collides with an existing clean schema key, the malformed schema is forced onto the counted suffix so the clean schema keeps the unsuffixed base name. - -# Validation - -- `dotnet format src\Refitter.slnx` -- `dotnet build -c Release src\Refitter.slnx` -- `dotnet test -c Release src\Refitter.slnx --no-build` (remaining failures were the known external-URL timeout lane in `PathParametersWithUrlTests`) -- `dotnet format --verify-no-changes src\Refitter.slnx` -- Manual CLI generation + scratch-project compilation for single-schema and collision fixtures - -# Parker Plan: Issue #1083 - -**Date:** 2026-05-01T14:04:19.681+02:00 -**Owner:** Parker -**Status:** Planning only - -## Decision - -Issue #1083 is a valid current bug in Refitter's generator pipeline. - - -- Reproduced at HEAD against `https://revenue-ie.github.io/paye-employers-documentation/PIT3/rest/paye-employers-rest-api-pit3.json`. -- Generated output contains both: - - `internal partial class` with no type name - - `Task<> LookUpERN(...)` -- The offending Swagger 2 definition key is `LookUpErnResponse.` and the `GET /ern/{employerRegistrationNumber}/{taxYear}` response references `#/definitions/LookUpErnResponse.` - -## Root Cause - -- Refitter currently relies on NSwag/NJsonSchema default type naming for schema names. -- NJsonSchema's `DefaultTypeNameGenerator` treats `.` as a segment separator and uses the last segment. -- For a trailing-dot name like `LookUpErnResponse.`, the last segment is empty, and the fallback path also returns an empty string instead of a usable anonymous name. -- That empty type name flows into both DTO emission and `generator.GetTypeName(...)`, producing the blank class declaration and empty generic return type. - -## Safest Fix Shape - -1. Add a Refitter-owned custom type-name generator that preserves current NSwag naming behavior for normal inputs. -2. Only special-case malformed hints whose final segment is empty or whose generated type name is blank. -3. Normalize those cases to the last non-empty segment (for example `LookUpErnResponse.` -> `LookUpErnResponse`) and still route the final identifier through Refitter-safe identifier sanitization. -4. Inject that generator in `CSharpClientGeneratorFactory` so the fix applies once at the DTO/type-resolution layer. - -## Main Files Likely Needed - -- `src\Refitter.Core\CSharpClientGeneratorFactory.cs` -- new core generator file such as `src\Refitter.Core\RefitterTypeNameGenerator.cs` -- possibly `src\Refitter.Core\IdentifierUtils.cs` if a shared helper is introduced for final identifier normalization -- regression coverage in `src\Refitter.Tests\Scenarios\PR1064BlockerRegressions.cs` or a dedicated scenario test file -- possibly `src\Refitter.Tests\IdentifierUtilsTests.cs` if helper behavior moves into `IdentifierUtils` - -## Risks / Tradeoffs - -- Any type-name generator change can rename emitted contracts for malformed schema names, so tests should pin current behavior for ordinary names. -- Avoid broad dot replacement: dotted names may already rely on NSwag's "use last segment" behavior, so the fix should target only empty-tail cases. -- Collisions remain possible if multiple malformed names normalize to the same identifier; rely on NSwag's reserved-name flow or add focused collision coverage. -- Rewriting schema keys and refs in the OpenAPI document would be much riskier because it can disturb references, exclusions, and other preprocessing logic. - - -# Squad Decisions - -## 2026-04-21 - -### PR #1067 Linked-Issue Closure Matrix - -**Lead:** Ripley -**Status:** REVIEWED - -- Treat **#1017, #1022, #1023, #1024, and #1026** as fully closed on the reviewed branch state. -- Treat **#1025** as **partial/documentation-first only**; do not auto-close it from PR wording. -- Final review guidance requires removing or downgrading `Fixes #1025` in the PR body so GitHub does not overstate closure. - -### Documentation and Package Guidance Alignment - -**Verified By:** Bishop / Ash -**Status:** REQUIRED AND VERIFIED - -- `Refitter.SourceGenerator` package guidance must describe Roslyn `AddSource()` behavior rather than legacy disk-file output. -- Consumer guidance must explicitly require a direct `Refit` reference (and `Refit.HttpClientFactory` when generated DI helpers are used). -- Disk-output settings (`outputFolder`, `contractsOutputFolder`, `generateMultipleFiles`) should be documented as CLI/MSBuild-oriented, not source-generator disk artifacts. -- Final safety-lane review approved PR #1067 once issue-closure wording was honest and the packaging/docs/test evidence aligned. - -### Session Directives Archived - -**By:** Christian Helle (via Copilot) - -- 2026-04-20: Commit changes as often as possible in small logical groups. -- 2026-04-21: Use Opus for all agents for the rest of that session only. -- 2026-04-21: Commit changes in small logical groups. -- 2026-04-25: Use GPT-5.5 for all agents for the rest of this session only. +### Refitter RefitterAutoScan MSBuild Implementation -## 2026-04-25 - -### Remaining Audit Matrix Pass (#1057) - -**Verified By:** Ripley -**Status:** VERIFIED - -- Treat **#1042** as **validation-only** until a concrete Spectre.Console.Cli parsing regression is reproduced at current HEAD. -- Treat **#1047** as **already fixed / stale issue text** at current HEAD because MSBuild now consumes CLI-emitted `GeneratedFile:` markers instead of regex-parsing `.refitter` JSON for output paths. -- Treat **#1056** as **doc/invariant-only** for now; preserve the current generation ordering/state flow and document the invariant before changing behavior. -- Treat **#1032** as **validation-first**; gather runtime evidence before changing enum-converter behavior. -- Coordination note from the verification pass: **#1045 and #1047 appear already fixed at HEAD**, and the remaining code-backed follow-up stays with Dallas/Parker. - -### Remaining Audit Repro Pass (#1057) - -**Verified By:** Lambert (Tester) -**Status:** EVIDENCE NARROWED - -- Treat **#1028** as **still reproducible by inspection** on current HEAD; the source-generator incremental pipeline still carries a `List` equality hazard. -- Treat **#1029** as **partial** on current HEAD; visible diagnostics improved, but the "no .refitter files found" path is still only `Debug.WriteLine`. -- Treat **#1033** as **still reproducible by inspection**; enum-converter injection still uses a hard-coded LF and needs newline normalization coverage. -- Treat **#1041** as **partial**; runtime resolution improved, but argument escaping and timeout kill semantics still leave repro surface. -- Treat **#1043** as **still reproducible**; legacy `--generate-authentication-header` bool-style CLI usage still fails at current HEAD. -- Treat **#1032, #1042, #1045, and #1047** as **validation-only / fixed-at-HEAD evidence** unless stronger failing repros appear. -- Treat **#1034, #1039, and #1056** as **not reproduced on current HEAD** in Lambert's pass. - -### Multi-spec Merge Collision Policy - -**Decided By:** Parker (Core Developer) -**Status:** APPROVED - -- `OpenApiDocumentFactory` should clone the first loaded document before merging additional specs so callers do not observe mutation of a previously loaded `OpenApiDocument`. -- Path and schema-key collisions across distinct OpenAPI inputs should fail fast with `InvalidOperationException` instead of silently keeping the first definition. -- Exact duplicate input paths should continue to deduplicate up front so feeding the same spec twice stays harmless. - -**Rationale:** Silent first-one-wins merge behavior hides real API-shape conflicts and only surfaces later during generation or runtime use. Failing fast is the safer core-library policy. - -### Core Lane Follow-up Gate (#1057) - -**Verified By:** Parker (Core Developer) -**Status:** FIXED / NARROWED - -- **#1033**: landed at HEAD with a code change in `src/Refitter.Core/RefitGenerator.cs` plus regression coverage in `src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs`. -- **#1032**: treat as validation-first pending review gate; no current core-lane code change required. -- **#1034** and **#1039**: treat as fixed-at-HEAD / no-repro on the reviewed branch state pending final gate review. -- **#1045**: treat as fixed-at-HEAD on the reviewed branch state pending final gate review. -- **#1056**: treat as doc/invariant-only for now; preserve current ordering behavior unless new failing evidence appears. -### Tooling Compatibility Follow-up - -**Verified By:** Dallas (Tooling Developer) -**Status:** APPROVED - -- Preserve CLI compatibility for `--generate-authentication-header` by treating the legacy boolean forms (`true`, `false`) and the bare flag as valid inputs. The bare flag and `true` now map to `AuthenticationHeaderStyle.Method`; `false` maps to `None`, while `Parameter` still requires the explicit enum value. -- Keep MSBuild runtime resolution resilient across both packed and test-project layouts. The task now prefers bundled framework-specific Refitter binaries, falls back to lower compatible TFMs when probing fails, and finally uses a co-located `refitter.dll` when the packaged layout is unavailable. - -### PR Prep Closure Guidance - -**Verified By:** Ripley -**Status:** DRAFTED FOR PR ASSEMBLY - -- Safe auto-close candidates on the reviewed branch state: **#1028**, **#1029**, **#1033**, and **#1043**. -- Keep **#1032**, **#1034**, **#1039**, **#1041**, **#1042**, **#1045**, **#1047**, and **#1056** out of PR auto-close wording until stronger evidence or final lane approval exists. -- **#1041** specifically remains a Dallas-owned tooling verdict before any PR body claims closure. - -### Ash core review of remaining #1057 closures - -**Verified By:** Ash -**Status:** REJECT - -- Verified acceptable: - - **#1032** does not reproduce the claimed custom `JsonNamingPolicy` override regression at current HEAD; runtime repro with a type-level `JsonStringEnumConverter` still serialized via `JsonSerializerOptions.Converters` (`"my_value"`). - - **#1045** is effectively fixed at current HEAD because `RefitGenerator.GetOpenApiDocument()` uses `OpenApiPaths` directly when populated instead of dereferencing `OpenApiPath`. - - **#1033** is the only intentional core code change in the working tree (`src/Refitter.Core/RefitGenerator.cs`) and it has matching regression coverage in `src/Refitter.Tests/Examples/InlineJsonConvertersTests.cs`. -- Still open / false closure: - - **#1034** remains open in `src/Refitter.Core/OpenApiDocumentFactory.cs:55-107`; `Merge()` still mutates `documents[0]` and still silently keeps the first path/schema on key collisions. - - **#1039** remains open in `src/Refitter.Core/ParameterExtractor.cs:447-487` plus `src/Refitter.Core/RefitInterfaceGenerator.cs:69-82`; `GetParameters()` removes query parameters from `operationModel.Parameters` before XML-doc generation reads the shared model. -- Follow-up requirement: reassign the remaining core revisions to Parker (or another core implementer) for real fixes before closing **#1034**/**#1039** from the `#1057` matrix. - -### Dallas core revision on rejected blockers - -**Verified By:** Dallas -**Status:** IMPLEMENTED / PENDING ASH RE-REVIEW - -- **#1034:** `OpenApiDocumentFactory.Merge()` now clones the first input before merge so callers no longer observe mutation of a previously loaded `OpenApiDocument`. -- **#1034:** duplicate path/schema collisions now emit warnings while preserving the existing merged entry; this revised pass does **not** follow the earlier fail-fast proposal. -- **#1039:** `ParameterExtractor` no longer mutates the shared `operationModel.Parameters` collection when building grouped query-parameter wrappers, so downstream consumers keep the original operation model intact. -- Regression coverage was refreshed for the revised core pass in `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs`, `src/Refitter.Tests/ParameterExtractorEdgeCaseTests.cs`, `src/Refitter.Tests/ParameterExtractorPrivateCoverageTests.cs`, and `src/Refitter.Tests/RegressionTests/Issue1039_DynamicQuerystringMutationTests.cs`. -- Dallas reported the revised core validation lane green; Ash is performing re-review and Lambert is reconciling the blocker-test lane against the landed behavior. - -### Ash core re-review of Dallas revision - -**Verified By:** Ash -**Status:** PARTIAL / BLOCKED - -- **#1039 resolved:** ParameterExtractor.GetParameters() no longer mutates operationModel.Parameters, and ParameterExtractorPrivateCoverageTests now lock that invariant for XML-doc generation and shared-model reuse. -- **#1034 still open:** OpenApiDocumentFactory.Merge() now clones the first input, but it still keeps the first conflicting path/schema/definition/security entry via Trace.TraceWarning(...) instead of failing fast. -- src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs still codifies warning-backed first-wins collision handling; the next narrow revision must flip that coverage to an InvalidOperationException contract for conflicting inputs. -- Do **not** close **#1034** from the #1057 matrix yet. Dallas owns one last narrow revision, and Lambert remains on the blocker-test lane. - -### Dallas final #1034 revision / Lambert blocker-test reconciliation - -**Verified By:** Dallas / Lambert -**Status:** IMPLEMENTED / READY FOR ASH FINAL GATE - -- `OpenApiDocumentFactory.Merge()` now preserves the clone-first non-mutation guarantee **and** fails fast with `InvalidOperationException` when distinct inputs introduce conflicting duplicate path, schema, definition, or security keys. -- Non-conflicting merges still return a new document without mutating either input document, and exact duplicate input paths remain harmless because they are deduplicated before merge. -- Blocker coverage is now aligned to the fail-fast contract in `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs`; `Issue1039_DynamicQuerystringMutationTests.cs` still preserves grouped-query XML-doc assertions across single-interface, `MultipleInterfaces.ByTag`, and `MultipleInterfaces.ByEndpoint` generation. -- Dallas's final narrow implementation pass is complete. Ash is performing the final review gate, and Lambert is reconciling the blocker-test lane against the landed fail-fast behavior. - -### Ash final core gate rejection - -**Verified By:** Ash -**Status:** REJECTED - -- **#1034** is still not proven closed: `src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs` only demonstrates fail-fast behavior for duplicate **paths**, not explicit conflicting **schemas**, **definitions**, and **security schemes**. -- The broader core validation lane is not green because `dotnet test -c Release src\Refitter.Tests\Refitter.Tests.csproj` still fails `Dynamic_Querystring_Generation_Preserves_Original_Query_Param_Documentation(ByEndpoint)` in `Issue1039_DynamicQuerystringMutationTests`. -- Dallas is now locked out of the next revision cycle for this artifact; Parker remains locked out from the prior rejected cycle. -- Lambert now owns the next/final revision cycle for **#1034** while staying in the blocker-test lane. - -### Ash final review of Lambert revision - -**Verified By:** Ash -**Status:** REJECTED - -- **#1039 acceptable:** `ParameterExtractor.GetQueryParameters()` still snapshots query parameters locally and preserves the shared `operationModel.Parameters` list; the regression coverage remains aligned with the intended non-mutating behavior. -- **#1034 still not proven closed:** the Swagger 2 definition-collision proof is still not isolated cleanly enough. `OpenApiDocumentFactoryMergeTests.Merge_With_Definition_Collision_Throws_And_Does_Not_Mutate_Inputs` still trips the duplicate **schema** conflict before it conclusively proves the duplicate **definition** lane. -- Remaining blocker for the next cycle: isolate the definition-specific fail-fast proof so the test fails for the intended definition-collision reason instead of the mirrored schema path. -- Lambert now joins Parker and Dallas in lockout for the next revision cycle on this artifact. -- Ripley now owns the next narrow revision cycle for **#1034**. - -### Ripley final #1034 proof-gap revision - -**Verified By:** Ripley -**Status:** IMPLEMENTED / PENDING ASH FINAL SIGNOFF - -- Preserve the source document schema type during clone/copy so Swagger 2 inputs stay on the intended definitions surface throughout merge handling. -- Isolate the Swagger 2 definition-collision proof at MergeIfMissingOrThrowOnConflict(...) so the definition-specific fail-fast contract is asserted directly instead of being masked by the mirrored schema collision first. -- Reported validation from the revision lane is green for dotnet build -c Release src\Refitter.slnx and dotnet test -c Release src\Refitter.Tests\Refitter.Tests.csproj. -- Ash now owns the final reviewer signoff before broader validation resumes. -### Ash final signoff on Ripley #1034/#1039 follow-up - -**Verified By:** Ash +**Leads:** Dallas (implementation), Bishop (documentation), Lambert (validation) **Status:** APPROVED -- **#1034 approved:** OpenApiDocumentFactory.Merge() now clones the first document before merge, fails fast on conflicting duplicate path/schema/definition/security keys, and isolates the remaining Swagger 2 definition proof through the shared MergeIfMissingOrThrowOnConflict(...) path. -- **#1039 approved:** grouped dynamic-query extraction still snapshots query parameters instead of mutating operationModel.Parameters, and XML-doc regression coverage remains locked for single-interface, ByTag, and ByEndpoint generation. -- Evidence reviewed: src/Refitter.Core/OpenApiDocumentFactory.cs, src/Refitter.Tests/OpenApiDocumentFactoryMergeTests.cs, src/Refitter.Tests/RegressionTests/Issue1039_DynamicQuerystringMutationTests.cs, and src/Refitter.Tests/ParameterExtractorPrivateCoverageTests.cs. -- Reviewer signoff was reported against dotnet test -c Release src\Refitter.Tests\Refitter.Tests.csproj with 1840 passing and 0 failing. - -### Final PR package guidance for #1057 - -**Prepared By:** Ripley -**Status:** READY FOR PR ASSEMBLY - -- Proposed PR title: `[v2.0 audit] Close remaining verified #1057 regressions`. -- Keep PR summary focused on five landed lanes: source-generator diagnostics, newline-safe enum-converter rewriting, non-mutating dynamic querystring generation, fail-fast multi-spec merge handling, and tooling/runtime compatibility hardening. -- Safe auto-close set for the final PR body: **#1028, #1029, #1033, #1034, #1039, #1041, #1043**. -- Keep **#1032, #1042, #1045, #1047, and #1056** out of auto-close wording because they are validation-only, fixed-at-HEAD/stale, or doc/invariant-only. -- Before opening the PR, recreate/publish v2.0.0-prerelease-fixes with `git push -u origin HEAD` because the local branch tracks a gone upstream. -- Latest local full validation reported: dotnet restore src\Refitter.slnx, dotnet build -c Release src\Refitter.slnx --no-restore, dotnet test -c Release src\Refitter.slnx --no-build, and dotnet format --verify-no-changes src\Refitter.slnx --no-restore with 1886 tests passing. - -### CLI help output assertions should be semantic - -**Verified By:** Lambert (Tester) -**Status:** APPROVED - -- src\Refitter\Program.cs intentionally rewrites a no-argument invocation to --help, exits 0, and emits Spectre.Console.Cli help output. -- The current product behavior is correct; the instability sits in whitespace-sensitive test expectations, not in production code. -- src\Refitter.Tests\GenerateCommandTests.cs should assert semantic help markers (usage pattern, sections, and known option names) instead of exact formatter-driven spacing/default-value layout. -- Validation reported: release run of src\Refitter.Tests\Refitter.Tests.csproj, focused rerun of Program_Main_Should_Show_Help_When_Invoked_Without_Arguments, and dotnet format --verify-no-changes src\Refitter.slnx. -- Cross-agent merge outcome: Dallas proved the Ubuntu failure was ANSI/wrapping noise in raw Spectre.Console help output, and Lambert landed the test-only fix in src\Refitter.Tests\GenerateCommandTests.cs by normalizing redirected console output before asserting semantic help markers. -- Final landed validation for the product commit `normalize help output test across platforms`: dotnet build -c Release src\Refitter.slnx, dotnet test -c Release src\Refitter.slnx, and dotnet format --verify-no-changes src\Refitter.slnx. - -### RefitterGenerateTask edge-case coverage stays test-only - -**Verified By:** Dallas / Lambert -**Status:** APPROVED - -- Preserve the current `src\Refitter.MSBuild\RefitterGenerateTask.cs` behavior and close the remaining coverage gap with regression tests instead of production changes. -- Lambert isolated the last uncovered branches to `TryExecuteRefitter()` exception handling, missing bundled CLI handling in `StartProcess()`, `ResolveRefitterDll()` fallback edges, sub-second timeout formatting, and the non-throwing `TryLogErrorFromException()` path. -- Dallas landed the test-only closure in `src\Refitter.Tests\RefitterGenerateTaskTests.cs`, covering blank package folders, whitespace runtime entries, co-located and first-bundled fallback resolution, missing bundled CLI failure, process-runner exception handling, millisecond timeout formatting, and successful `LogErrorFromException` forwarding. -- Reported validation: `dotnet test --project src\Refitter.Tests\Refitter.Tests.csproj -c Release --coverage --coverage-output coverage.cobertura.xml --coverage-output-format xml`, `dotnet build -c Release src\Refitter.slnx --no-restore`, and `dotnet format --verify-no-changes src\Refitter.slnx --no-restore`. -- Result: `RefitterGenerateTask.cs` reached 100% line coverage, 100% block coverage, and 0 partial functions in the reported coverage output. -### PR #1070 SonarCloud quality-gate repair - -**Verified By:** Dallas / Ash / Parker -**Status:** APPROVED - -- Keep the ParameterExtractor (S1066) and RefitterGenerateTask (S3267, S3358) changes narrow and behavior-preserving; Ash approved those cleanups as safe. -- Keep the source-generator diagnostic cleanup on the stable one-descriptor-per-ID contract: reuse the shared Refitter title/category constant, but assign distinct IDs when the title/message semantics differ. -- Preserve GeneratedDiagnostic as a readonly record struct; retain the explicit ordinal GetHashCode() behavior and handle Sonar S1206 with a targeted suppression plus justification instead of rewriting the type into a manual struct. -- Final validation recorded for the landed artifact: dotnet build -c Release src\Refitter.slnx --no-restore, dotnet test -c Release --solution src\Refitter.slnx --no-build, and dotnet format --verify-no-changes src\Refitter.slnx --no-restore. - -## 2026-04-26 - -### AI-slop cleanup sequencing - -**Verified By:** Ripley -**Status:** APPROVED - -- Start with **docs/help drift** so stale SourceGenerator and MSBuild guidance is corrected before code refactors. -- Follow with **settings/spec-path normalization** across CLI and source-generator entry points, then centralize the shared **`GeneratedFile:` marker contract**. -- Keep **test-surface cleanup** ahead of deeper generator dedup so regression coverage is stronger before touching contract-sensitive code. -- Leave **core generator dedup** for the last batch and treat `OpenApiDocumentFactory` merge semantics, auth-header CLI compatibility, source-generator diagnostic contracts, and single-file `GeneratedCode` metadata behavior as reviewer-gated cleanup seams. -- Current squad capacity is sufficient for docs, tests, tooling, and settings cleanup; bring in a specialized C# reviewer only for the later core generator / `ParameterExtractor` dedup lane. - -### Cleanup-safe baseline and coverage guardrails - -**Verified By:** Lambert (Tester) -**Status:** VERIFIED - -- Baseline validation is healthy on this machine: restore, release build, solution tests, and format verification all passed. -- Treat the Codecov command in `.github\workflows\codecov.yml` as the authoritative cleanup coverage lane; it targets `src\Refitter.Tests\Refitter.Tests.csproj`. -- Prefer narrow branch-coverage gains before production edits in `src\Refitter.Core\ContractTypeSuffixApplier.cs`, `SchemaCleaner.cs`, `CSharpClientGeneratorFactory.cs`, `XmlDocumentationGenerator.cs`, `IdentifierUtils.cs`, and `src\Refitter\Settings.cs`. -- Do not use the live-network test surface (`OpenApiDocumentFactoryTests`, `SwaggerPetstoreTests`, `SwaggerPetstoreApizrTests`, `Examples\OpenApiUrlTests`) as a stability signal during cleanup because it remains environment-sensitive. -- Coverage reports also include `.test-work` runtime-proof artifacts; note them, but prioritize real repository source files first. - -### Generator cleanup phasing - -**Verified By:** Parker (Core Developer) -**Status:** APPROVED - -- Land generator cleanups in two phases: first trim redundant string/branch duplication already covered by public regressions in `XmlDocumentationGenerator.AppendXmlCommentBlock()` and `ContractTypeSuffixApplier.TypeSuffixRewriter`. -- Before deeper dedupe, add or tighten compile-backed public regressions for `ParameterExtractor`, multipart generation, and interface-emission behavior. -- Route any cleanup that deduplicates `RefitGenerator.Generate()` vs `GenerateMultipleFiles()` or the shared method-emission logic across the three interface generators through **Ash review**. - -### AI-slop safety review gate - -**Verified By:** Ash -**Status:** REQUIRED - -- Treat the safety-sensitive generator and MSBuild cleanup lane as **validation-first** rather than style-first. -- Require source-generator regressions proving same-directory `.refitter` files with the same `outputFilename` do not collide on `AddSource()` hint names and that `AdditionalText.GetText(...) == null` reports a diagnostic instead of throwing. -- Require generator regressions for OpenAPI-title-derived identifiers containing `<` / `>` and for ByEndpoint interface names whose internal `I` characters must remain intact. -- Require MSBuild coverage for `GetInstalledDotnetRuntimes()` timeout/failure handling before claiming the process-launch path is hardened. - -### Session directives refresh - -**By:** Christian Helle (via Copilot) - -- 2026-04-26: Have all agents use GPT-5.5 for the rest of this session only. -- 2026-04-26: Commit changes as frequent as possible in small logical groups for a detailed progress history. - -## 2026-04-28 - -### e-conomic Multi-Spec OpenAPI Merge Failure - -**Leads:** Ripley (analysis), Parker (core finding), Dallas (tooling validation) -**Tester:** Lambert -**Status:** TRIAGED AND APPROVED FOR FIX - -#### Findings - -- `test\economic.refitter` triggers merge failure in `OpenApiDocumentFactory.Merge()` while combining `economic-products.json` and `economic-webhooks.json`. -- Exception: `InvalidOperationException: Cannot merge OpenAPI documents because a duplicate schema 'Error' was found.` -- Root cause: `AreEquivalent()` calls `Serializer.Serialize()` on NSwag/NJsonSchema objects; object-cycle behavior returns `false` for semantically identical schemas. -- Both `Error` and `ProblemDetails` are duplicated and textually equivalent in source JSON; merge fails at `Error` first. -- Issue is **not** in `.refitter` parsing, path resolution, or individual spec generationβ€”each spec generates successfully in isolation. -- CLI, source generator, and MSBuild all reach the same merge failure point before code output. - -#### Fix Ownership and Scope - -**Primary owner:** `Refitter.Core` β†’ `src\Refitter.Core\OpenApiDocumentFactory.cs` β†’ `MergeIfMissingOrThrowOnConflict()` / `AreEquivalent()` - -- Replace `AreEquivalent()` with OpenAPI-aware semantic equivalence check (use NSwag `ToJson()` or canonical JSON representation). -- Keep fail-fast policy for **genuinely conflicting** duplicate path/schema/definition/security entries. -- Keep clone-first/non-mutating merge behavior. -- Do **not** edit e-conomic OpenAPI specs; do **not** rename schemas in merge. - -#### Approved Test Coverage and Gates - -1. Add `OpenApiDocumentFactoryMergeTests`: - - `Merge_With_Equivalent_Duplicate_Schema_Does_Not_Throw` β€” OpenAPI 3 docs with identical schema key and body; both paths preserved. - - Preserve existing `Merge_With_Schema_Collision_Throws_And_Does_Not_Mutate_Inputs` for conflicts. - - Optional e-conomic-shaped regression for `Error` schema with descriptions, nullable properties, extensions, `additionalProperties: false`. - -2. Compile-backed regression: two specs with identical common error schema and distinct paths; assert generation succeeds and generated code builds. - -3. Run generated-code validation for `test\economic.refitter` after merge fix; triage any post-merge generation failure separately. - -4. Existing merge-collision tests must continue passing. - -#### Constraints - -- Preserve relative-path behavior across CLI and source generator. -- Keep `openApiPaths` semantics consistent. -- OpenAPI 3 `components.schemas` and Swagger 2 `definitions` equivalence must remain aligned. -- Do not hide genuine schema differences in comparison. - -### Ash economic merge review - -- Date: 2026-04-28T15:21:48.369+02:00 - -Verdict: APPROVED - -Rationale: -- `OpenApiDocumentFactory.Merge()` still fails fast on genuinely conflicting duplicate paths, schemas, Swagger 2 definitions, and security schemes while accepting equivalent duplicate schemas. -- Equivalence is based on normalized OpenAPI/NSwag/NJsonSchema JSON tokens rather than object identity; the recursive schema test covers cyclic schema graphs and the e-conomic regression verifies real multi-spec generation remains buildable. -- Focused validation passed for `OpenApiDocumentFactoryMergeTests`, `Issue1016_MultiSpecSchemaMergeTests`, and the duplicate-path merge coverage via the net10.0 TUnit executable. A first `dotnet test --filter` attempt failed because this TUnit project does not support that option, not because of product/test failure. - -### Lambert e-conomic validation - -- Date: 2026-04-28T15:21:48.369+02:00 -- Validation result: PASS -- Evidence: - - `dotnet build -c Release src\Refitter.slnx` passed. - - Focused `OpenApiDocumentFactoryMergeTests` passed: 15 succeeded / 0 failed. - - Focused `Issue1016_MultiSpecSchemaMergeTests` passed: 7 succeeded / 0 failed. - - `dotnet test -c Release src\Refitter.slnx` passed. - - `dotnet format --verify-no-changes src\Refitter.slnx` passed. - - `dotnet .\src\Refitter\bin\Release\net9.0\refitter.dll --settings-file test\economic.refitter --no-banner --no-logging --simple-output` passed and generated `test\GeneratedCode\economic.cs`. -- Decision signal: the e-conomic duplicate equivalent schema merge path is validated by focused regression tests, full solution tests, and real CLI generation. - -### Parker: e-conomic equivalence implementation - -- Date: 2026-04-28T15:21:48.369+02:00 -- Decision: `OpenApiDocumentFactory.AreEquivalent()` should compare normalized OpenAPI/NSwag JSON first and use a narrow NJsonSchema structural fallback only for schemas whose standalone JSON serialization cannot resolve references. -- Rationale: duplicate schemas from separate vendor specs can be semantically identical while NJsonSchema object graphs contain unresolved/cyclic references; raw serializer comparison falsely reports conflicts. -- Guardrail: merge still copies missing keys, ignores equivalent duplicates, and throws `InvalidOperationException` for non-equivalent duplicate paths, schemas, definitions, and security schemes. -- Validation: focused merge tests and Issue1016 multi-spec regression tests passed. - -### User directive: 100% coverage on OpenApiDocumentFactory - -**By:** Christian Helle (via Copilot) -**Date:** 2026-04-28T19:19:33.388+02:00 - -Get `src/Refitter.Core/OpenApiDocumentFactory.cs` to 100% coverage; it is one of the most important parts of the code base and all possibilities should be tested thoroughly. - -### Ripley: GitHub Issue/PR Prep for e-conomic Multi-Spec Fix - -**Author:** Ripley (Lead) -**Date:** 2026-04-28 -**Status:** READY FOR EXECUTION ON WORDING - #### Decision -For this repo, the pending issue + PR sequence should target **`main`**, not `dev`, because the live remote has no `dev` branch and GitHub reports `main` as the default branch. - -The current execution branch is **`economic-openapi`**. It is already pushed, tracks `origin/economic-openapi`, has no open PR, and carries the seven fix commits for the e-conomic multi-spec merge work. - -`test/multiple-sources.refitter` does **not** currently block PR creation. It is already committed on the branch and the working tree is clean, so no extra stash/reset step is required before creating the issue and PR. +Gate Refitter's automatic MSBuild hook behind a new wrapper target instead of conditioning RefitterGenerate directly. #### Why -- Repo reality overrides the generic dev-first squad workflow here. -- Using `main` avoids opening a PR against a non-existent branch. -- A clean working tree means the PR can safely describe only the already-committed fix set. - -#### Execution Notes - -1. Create the GitHub issue once Bishop provides final wording. -2. Re-push `economic-openapi` only if a last-minute commit is added after wording arrives. -3. Open the PR from `economic-openapi` to `main` and include `Closes #` in the PR body. - -### Lambert: OpenApiDocumentFactory 100% Coverage - -**Owner:** Lambert -**Date:** 2026-04-28 - -#### Decision - -Treat `NSwag.OpenApiDocument.Tags` and `NSwag.OpenApiDocument.Components.Schemas` as non-null collections inside `OpenApiDocumentFactory.Merge`, and use collection-count checks instead of null-coalescing/null-conditional branches there. - -#### Why - -Local reflection and coverage evidence showed the previous null-handling branches in `Merge` were not reachable for parsed NSwag documents because: - -- `Tags` materializes as an empty list, not `null` -- `Components` materializes as a non-null object -- `Components.Schemas` materializes as an empty dictionary, not `null` - -Those unreachable branches blocked `OpenApiDocumentFactory.cs` from reaching 100% coverage even after adding focused regression tests. +- RefitterGenerate must remain callable through dotnet build -t:RefitterGenerate even when false. +- A separate _RefitterGenerateOnBuild target keeps the on-build behavior opt-out while preserving the explicit target contract for CI and local workflows. +- The workflow proof asserts both the absence of task execution during normal clean builds and the continued success of explicit generation with auto-scan disabled. #### Evidence -- Full coverage command passed and reported `OpenApiDocumentFactory` functions at 100% line/block coverage in `src\Refitter.Tests\bin\Release\net10.0\TestResults\coverage.cobertura.xml` -- Reflection check confirmed NSwag parses both OpenAPI 3 and Swagger 2 documents with non-null `Tags`, `Components`, and `Components.Schemas` - -#### Impact - -- No intended behavior change for real parsed OpenAPI inputs -- Merge logic is simpler and now fully coverable with focused regression tests - -## 2026-04-29 - -### Ash: OpenApiDocument clone path should use parameterless ToJson() - -**Author:** Ash -**Date:** 2026-04-29T10:41:29.997+02:00 - -#### What - -For `src\Refitter.Core\OpenApiDocumentFactory.cs`, replace the obsolete `OpenApiDocument.ToJson(SchemaType)` clone path with `OpenApiDocument.ToJson()` followed by `OpenApiDocument.FromJsonAsync(...)`. - -#### Why - -Focused coverage in `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs` shows the non-obsolete serializer preserves `SchemaType` and the merge-critical collections for embedded Swagger 2 and OpenAPI 3 fixtures, while mixed-version merge assertions confirm the cloned base document still serializes with the correct top-level version marker. That keeps the clone-first/fail-fast merge contract intact without carrying the obsolete API forward. - -### Lambert: OpenApi clone validation surface +- src\Refitter.MSBuild\Refitter.MSBuild.targets defines RefitterGenerate as the explicit generation target and _RefitterGenerateOnBuild as the build hook with BeforeTargets="CoreCompile", DependsOnTargets="RefitterGenerate", and Condition="'$(RefitterAutoScan)' != 'false'". +- src\Refitter.MSBuild\README.md now states that RefitterGenerate remains the on-demand target, _RefitterGenerateOnBuild is the normal-build hook, and RefitterAutoScan controls that wrapper target. +- docs\docfx_project\articles\msbuild.md now describes the same target/property relationship and preserves dotnet build -t:RefitterGenerate as the manual invocation path. +- .github\workflows\msbuild.yml updated with RefitterAutoScan tests. +- Full validation passed: restore, build, test, and format verification. -**Author:** Lambert -**Date:** 2026-04-29T10:41:29.997+02:00 - -#### What - -Validate the obsolete `OpenApiDocument.ToJson(SchemaType)` replacement through focused `OpenApiDocumentFactoryMergeTests` that round-trip embedded Swagger Petstore JSON fixtures for both Swagger 2 and OpenAPI 3, then assert merge keeps the base document schema type. - -#### Why - -The obsolete call only exists to clone the first `NSwag.OpenApiDocument` before merge. The safest proof is not a full multi-spec integration run first; it is a narrow clone/merge surface that confirms the cloned document keeps `SchemaType` and the key collections (`Paths`, `Tags`, `Components.Schemas`, `Definitions`, `SecurityDefinitions`) that merge relies on. - -### User directive: Use GPT-5.5 for agents +### User Directive **By:** Christian Helle (via Copilot) -**Date:** 2026-04-29T10:41:29.997+02:00 +**Date:** 2026-05-29T12:44:43.282+02:00 +**What:** Use claude-opus-4.8 for all agent work for the rest of this session only. +**Why:** User request β€” captured for team memory -Use GPT-5.5 for all agents for the rest of this session only. -### Ripley PR Assembly (PR #1079) +## 2026-05-29T14:24:16.307+02:00 - RefitterAutoScan build gating -**Author:** Ripley (Lead) -**Date:** 2026-04-29T11:51:36.530+02:00 -**Branch:** `build-warnings` -**PR:** #1079 `Tighten warning handling across Refitter builds` -**Status:** READY FOR REVIEW +- Context: issue #1094 needs MSBuild users to disable automatic `.refitter` scanning during ordinary builds without losing the explicit `RefitterGenerate` entry point. +- Decision: keep `RefitterGenerate` as the standalone generation target, default `RefitterAutoScan` to `true`, and move the normal-build hook into a separate target that depends on `RefitterGenerate` only when `RefitterAutoScan` is not `false`. +- Consequence: `dotnet build -t:RefitterGenerate -p:RefitterAutoScan=false` still generates code on demand, while later `dotnet build -p:RefitterAutoScan=false` runs compile against the already-generated `.cs` files without re-running Refitter. -#### Decision -Frame the PR around the branch's real aggregate scope instead of only the newest OpenAPI clone-warning fix. +## 2026-05-29T14:24:16.307+02:00 - Lambert validation for issue #1094 -#### Rationale +- Context: Christian Helle requested validation of the real working-tree implementation for the RefitterAutoScan behavior change in issue #1094. +- Decision: approve the current working-tree implementation as matching the requirement. +- Evidence: the actual diff is present in the four expected files; `src\Refitter.MSBuild\Refitter.MSBuild.targets` defaults `RefitterAutoScan` to `true` and gates normal builds through `_RefitterGenerateOnBuild`; local validation passed for auto-scan enabled builds, explicit `dotnet build -t:RefitterGenerate -p:RefitterAutoScan=false`, and normal `dotnet build -p:RefitterAutoScan=false` without `RefitterGenerateTask`; README, docfx docs, and `.github\workflows\msbuild.yml` describe and verify the same behavior. -- The branch includes multiple warning-focused commits across core, tests, CLI packaging metadata, nullable-flow cleanup, and warning enforcement changes. -- It already carries a committed `.squad` decision/history sync. -- The obsolete `OpenApiDocument` clone fix remains important and should stay called out explicitly, but only as one bullet within the broader warning-hardening frame. +## 2026-05-29T15:07:47.442+02:00 - Commit grouping for issue #1094 -#### Reviewer Guidance +- Context: the issue #1094 working tree already contained the product changes across MSBuild targets, CI coverage, and user-facing docs, plus a stale `.squad\commit-msg.txt` deletion from earlier automation. +- Decision: group the product diff into two user-facing commits (`tooling or CI` and `docs`) and keep the stale automation cleanup plus Squad bookkeeping in a separate housekeeping commit with no co-author trailer. +- Consequence: reviewers can reason about behavior changes separately from documentation and repository hygiene, while the branch still finishes with a clean working tree. -- Review against `main`. -- Expect main product deltas in `src\Refitter.Core\OpenApiDocumentFactory.cs`, `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs`, `src\Refitter.Core\Refitter.Core.csproj`, `src\Refitter.Tests\Refitter.Tests.csproj`, and `src\Refitter\Refitter.csproj`. -- Treat the `.squad` history/decision sync as a carried branch artifact, not the headline behavior change. diff --git a/.squad/skills/commit-grouping-hygiene/SKILL.md b/.squad/skills/commit-grouping-hygiene/SKILL.md new file mode 100644 index 000000000..1a019e3e3 --- /dev/null +++ b/.squad/skills/commit-grouping-hygiene/SKILL.md @@ -0,0 +1,26 @@ +--- +name: "commit-grouping-hygiene" +description: "Split landed work into focused commits and isolate stale automation cleanup" +domain: "git, review" +confidence: "high" +source: "earned" +last_updated: "2026-05-29T15:07:47.442+02:00" +--- + +## Context + +Use this when a working tree already contains a completed feature or fix plus docs and leftover automation artifacts, and your job is to turn that diff into reviewable commits without rewriting history. + +## Pattern + +- Inspect the whole diff before staging anything so you can group files by behavior lane instead of by directory alone. +- Put executable behavior changes first (for example tooling targets and CI proof), then documentation that explains the shipped behavior. +- If the branch contains stale automation debris such as `.squad\commit-msg.txt`, remove it in a final housekeeping commit together with any required Squad history or decision updates. +- Keep housekeeping out of the product commits unless the cleanup is required for the feature to function. +- Use direct `git add ` and `git commit -m ""` commands so the grouping stays deterministic and non-interactive. + +## Anti-patterns + +- Mixing docs, product logic, and Squad bookkeeping into one commit when the diff naturally separates. +- Leaving temporary commit-message files or other automation leftovers dangling after the product commits are created. +- Amending or force-rewriting commits when the task only requires focused grouping of the current working tree. diff --git a/docs/docfx_project/articles/msbuild.md b/docs/docfx_project/articles/msbuild.md index 7a89075ce..b31753649 100644 --- a/docs/docfx_project/articles/msbuild.md +++ b/docs/docfx_project/articles/msbuild.md @@ -57,18 +57,37 @@ To use the package, install `Refitter.MSBuild` The MSBuild package includes a custom `.target` file which executes the `RefitterGenerateTask` custom task and looks something like this: ```xml + + true + - + + DisableLogging="$(RefitterNoLogging)" + SkipValidation="$(RefitterSkipValidation)" + IncludePatterns="$(RefitterIncludePatterns)"> + ``` -The `RefitterGenerateTask` task will scan the project folder for `.refitter` files and executes them all. By default, telemetry collection is enabled, and to opt-out of it you must specify `true` in the `.csproj` `` \ No newline at end of file +The `RefitterGenerateTask` task scans the project folder for `.refitter` files and executes them all. `RefitterAutoScan` defaults to `true`, so regular builds keep their current behavior. Set `false` to disable the automatic build hook while still allowing explicit generation through `dotnet build -t:RefitterGenerate`. + +By default, telemetry collection is enabled, and to opt-out of it you must specify `true` in the `.csproj` ``. + +```xml + + false + +``` + +When `RefitterAutoScan` is `false`, run `dotnet build -t:RefitterGenerate` whenever you want MSBuild to regenerate code from `.refitter` files without re-enabling generation on every normal build. After that explicit generation step, regular `dotnet build` invocations can reuse the generated `.cs` files without re-running the Refitter task. \ No newline at end of file diff --git a/global.json b/global.json index 7f04ecb68..5a3fc6f2d 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "10.0.201", + "version": "10.0.100", "rollForward": "latestFeature", "testingPlatform": true }, diff --git a/src/Refitter.MSBuild/README.md b/src/Refitter.MSBuild/README.md index bea642535..88582ae1b 100644 --- a/src/Refitter.MSBuild/README.md +++ b/src/Refitter.MSBuild/README.md @@ -13,22 +13,30 @@ dotnet add package Refitter.MSBuild The MSBuild package includes a custom `.target` file which executes the `RefitterGenerateTask` custom task and looks something like this: ```xml + + true + - + + SkipValidation="$(RefitterSkipValidation)" + IncludePatterns="$(RefitterIncludePatterns)"> + ``` -The `RefitterGenerateTask` task will scan the project folder for `.refitter` files and execute them all. +The `RefitterGenerateTask` task scans the project folder for `.refitter` files and executes them all. `RefitterAutoScan` defaults to `true`, so normal builds keep generating code automatically. Set `false` to skip that automatic build hook while keeping `dotnet build -t:RefitterGenerate` available for explicit generation. ## Configuration @@ -48,6 +56,16 @@ You can also skip OpenAPI validation by setting: ``` +To disable automatic scanning during normal builds, but still allow explicit generation, set: + +```xml + + false + +``` + +Then run `dotnet build -t:RefitterGenerate` whenever you want Refitter to scan the project and regenerate code on demand. After that explicit generation step, regular `dotnet build` invocations can reuse the generated `.cs` files without re-running the Refitter task. + ## Example Create a `.refitter` file in your project: @@ -60,7 +78,7 @@ Create a `.refitter` file in your project: } ``` -Now, every time you build your project, Refitter will automatically generate the API client code based on your OpenAPI specification. +Now, every time you build your project, Refitter will automatically generate the API client code based on your OpenAPI specification unless you set `RefitterAutoScan` to `false`. ### .Refitter File format diff --git a/src/Refitter.MSBuild/Refitter.MSBuild.targets b/src/Refitter.MSBuild/Refitter.MSBuild.targets index 5fe005670..90ce865d0 100644 --- a/src/Refitter.MSBuild/Refitter.MSBuild.targets +++ b/src/Refitter.MSBuild/Refitter.MSBuild.targets @@ -1,10 +1,14 @@ + + true + + - + + +