Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
9e2ac20
docs: Add migration guidance for OpenAPI parser upgrade (v1.x → v3.x)
christianhelle Apr 20, 2026
a4218ee
fix(msbuild): track actual outputs
christianhelle Apr 20, 2026
93dc911
fix(source-generator): hide transitive deps
christianhelle Apr 20, 2026
bff64bb
fix(core): repair AOT context generation
christianhelle Apr 20, 2026
5b8bb06
fix(core): keep optional shapes explicit
christianhelle Apr 20, 2026
bb37c50
docs(squad): capture tooling pattern
christianhelle Apr 20, 2026
9f9fb67
fix(core): harden serializer context helpers
christianhelle Apr 20, 2026
4abfa3c
test: cover Swagger2 #1026 opt-in
christianhelle Apr 20, 2026
dee7bde
docs(squad): record audit p1 learnings
christianhelle Apr 20, 2026
03d6734
docs: add upgrade note highlighting breaking changes v2.0.0
christianhelle Apr 20, 2026
3b25917
fix(source-generator): prevent Refit transitive leak with PrivateAsse…
christianhelle Apr 20, 2026
97bcacb
docs(squad): record #1024 resolution - transitive dependency leak fixed
christianhelle Apr 20, 2026
92dc250
test(aot): prove polymorphic context runtime
christianhelle Apr 20, 2026
e9a3c21
Update squad state
christianhelle Apr 20, 2026
7b68b69
fix: resolve build errors - path normalization, PrivateAssets test, a…
Copilot Apr 21, 2026
8fbbb3d
test(core): cover context formatting edges
christianhelle Apr 21, 2026
c762eb5
test(tooling): prove marker and package flow
christianhelle Apr 21, 2026
60e2d51
docs: clarify migration and generator usage
christianhelle Apr 21, 2026
b4e32d7
chore(squad): update agent learnings
christianhelle Apr 21, 2026
9b83f80
Fix failing tests
christianhelle Apr 23, 2026
8fe4b50
Fix failing test
christianhelle Apr 23, 2026
0bb48d1
Fix smoke tests
christianhelle Apr 23, 2026
6eea581
fix(test): avoid source generator pack race
christianhelle Apr 24, 2026
f5ab7ca
test(coverage): close remaining gaps
christianhelle Apr 24, 2026
29d9f0b
test(coverage): fix patch coverage gaps
christianhelle Apr 24, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions .squad/agents/ash/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,11 @@
- Added to the squad on 2026-04-20 as a specialist reviewer for PR #1064 review work.
- PR #1064's Roslyn rewrite fixes raw regex corruption for ContractTypeSuffix, but issue #1013 is not closed unless it also blocks suffix-target collisions like `Pet` + `PetDto`.
- For ParameterExtractor multipart fields, deduplication must use the emitted C# identifier, not the original OpenAPI property key, or issue #1018 still reproduces through sanitization collisions.
- For source-generator dependency reviews, inspect the packed `.nuspec` and `analyzers/dotnet/cs/` payload, not just the `.csproj`; this repo's package bundles `OasReader.dll` as an analyzer asset but still declares `Refit` as a transitive NuGet dependency.
- `RuntimeCompatibilityTests.Does_Not_Auto_Enable_Optional_Properties_As_Nullable_When_NRT_Enabled_Swagger2` currently fails because Swagger 2 generation still emits `string?` for optional properties when only `GenerateNullableReferenceTypes=true`.
- `JsonSerializerContextGenerator` now statically covers nested types, closed generic usages, cross-namespace qualification, and `I`-prefix stripping; polymorphic specs also register all derived DTOs in the generated serializer context, but there is still no runtime AOT serialization regression test for that path.
- The minimal safe #1026 fix lives in `src/Refitter.Core/RefitGenerator.cs`: for Swagger 2 only, when NRT is enabled but `GenerateOptionalPropertiesAsNullable` is false, a Roslyn rewrite strips `?` from generated nullable reference-type property declarations after NSwag generation.
- Focused #1026 regression coverage now includes Swagger 2 explicit opt-in in `src/Refitter.Tests/Examples/RuntimeCompatibilityTests.cs`, and the four targeted NRT/nullability tests pass against the updated core/test binaries.

## 2026-04-20 Update: PR #1064 Review Complete

Expand Down Expand Up @@ -181,3 +186,42 @@ When fixing parameter extraction/deduplication:
**Final Session Log:** `.squad/log/2026-04-20T16-00-14Z-pr1064-blocker-fixes.md`

**Merge Status:** ✅ APPROVED (cleanup of test JSON files required)

## 2026-04-20 Update: Issue #1024 Transitive Dependency Leak - FULLY RESOLVED

**Task:** Own revision #2 for #1024 after Dallas lockout; complete the packaging fix
**Status:** ✅ COMPLETE — Transitive dependency leak eliminated; package and docs aligned
**Lockout Context:** Dallas attempted fix at commit 20ab08de with `PrivateAssets="compile"` but this was insufficient

**Root Cause Analysis:**
Dallas set `PrivateAssets="compile"` on Refit reference in `Refitter.SourceGenerator.csproj`. This prevents Refit assemblies from flowing into the generator's compilation but does NOT prevent NuGet from adding Refit as a transitive dependency in the packed `.nupkg` file.

**Evidence:**
Packed the source generator and examined the `.nuspec` file inside the `.nupkg`:
- **Before fix (PrivateAssets="compile"):** nuspec contained `<dependency id="Refit" version="10.1.6" include="Runtime,Build,Native,ContentFiles,Analyzers,BuildTransitive" />`
- **After fix (PrivateAssets="all"):** nuspec contains `<group targetFramework=".NETStandard2.0" />` with zero dependencies

**Fix Applied:**
Changed `Refitter.SourceGenerator.csproj` line 22:
```diff
- <PackageReference Include="Refit" Version="10.1.6" PrivateAssets="compile" />
+ <PackageReference Include="Refit" Version="10.1.6" PrivateAssets="all" />
```

**Documentation Alignment:**
- README.md already documented at lines 559-564 that consumers must add explicit Refit reference
- Updated `docs/docfx_project/articles/source-generator.md` to match README guidance
- Both now clearly state: "The source generator no longer upgrades Refit transitively"

**Verification:**
1. Built source generator project cleanly (0 errors)
2. Packed as `.nupkg` and extracted to verify nuspec dependencies section
3. Confirmed zero transitive dependencies in package metadata
4. Verified documentation alignment across README and docs/

**Commit:** `3ebbc5df` — "fix(source-generator): prevent Refit transitive leak with PrivateAssets=all"

**Issue #1024 Status:** ✅ CLOSED — No transitive dependency leak; explicit Refit reference requirement documented

**Key Learning:**
`PrivateAssets="compile"` only prevents assembly references from flowing to the consuming compilation. To prevent NuGet package dependencies from appearing in the `.nuspec`, you must use `PrivateAssets="all"`. For source generators and analyzers that generate code requiring runtime dependencies (like Refit), the consuming project must add those dependencies explicitly.
2 changes: 2 additions & 0 deletions .squad/agents/bishop/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,5 @@
- **Breaking Changes Finalization (2026-04-17):** Created finalized discussion draft in session files (not published to repo) as pre-release heads-up. Created permanent migration guide in docfx articles with structured Impact → Migration → Evidence format. Updated toc.yml to make guide discoverable after Source Generator. Used 2026-04-17 squad consensus as authoritative source (overriding Dallas's initial audit). Scoped guidance to Refitter-owned changes only (excluded non-breaking improvements, ecosystem guidance, and the MSBuild output path bug fix from core migration narrative). Documented team decision in decisions/inbox for squad alignment.
- **Guidance Review & Approval (2026-04-18):** Ripley completed final review of all artifacts. ✅ APPROVED FOR PUBLICATION. Identified key pattern: breaking change communication requires dual artifacts (discussion + docs). Discussion draft remains in session state pending user publication decision. Migration guide and TOC updates ready for merge.
- **PR #1064 Documentation Review (2026-04-20):** Verified that 29 closed issues are real code fixes, not just documentation. Breaking-changes guide is accurate for the 2 intentional breaks (auth rename, source generator disk files). However, P0/P1 silent bug fixes (hint-name collisions, MSBuild swallowing errors, XML injection, Swagger 2.0 NRE, multi-spec merge, etc.) are not mentioned in user-facing docs. CLI --output precedence fix (#1021) needs README clarification. Security fix #1035 (XML escaping) warrants explicit migration note. Recommendation: 92% documentation complete; ready to merge with 3 follow-up items for README enhancement pre-release.
- **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."
7 changes: 7 additions & 0 deletions .squad/agents/dallas/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

- Team initialized on 2026-04-16.
- **Issue #998 findings (2026-04-16):** Validated MSBuild tooling path. CLI loads settings correctly (naming honored). Single-file output without explicit `outputFilename` falls back to `Output.cs` and skips `./Generated` folder when it matches default. Real product bug: MSBuild expects wrong file locations on first clean build.
- **PR #1067 tooling proof (2026-04-21):** The strongest regression signal came from testing the actual stdout marker contract and the packed `Refitter.SourceGenerator` artifact. `RefitterGenerateTask` now has unit coverage for exact include matching plus duplicate/zero-marker handling, and package validation inspects the produced `.nupkg`/`.nuspec` instead of trusting project metadata.

### 2026-04-20: PR #1064 Tooling Review

Expand Down Expand Up @@ -267,3 +268,9 @@ dotnet test --project src/Refitter.Tests/Refitter.Tests.csproj -c Release --no-r

**Merge Status:** ✅ APPROVED (temporary test JSON files marked for deletion)

### 2026-04-20: P1 Tooling Fixes (#1022, #1023, #1024)

- **MSBuild generated-file discovery:** `src\Refitter.MSBuild\RefitterGenerateTask.cs` now trusts `GeneratedFile:` markers emitted by `src\Refitter\GenerateCommand.cs --simple-output` instead of re-parsing `.refitter` contents. This removes duplicated output-path prediction logic and keeps MSBuild compile items aligned with the CLI's actual writes.
- **MSBuild include filtering semantics:** `RefitterIncludePatterns` now matches only exact filenames, exact project-relative paths, or exact full paths. Substring matching was removed; `apis\petstore.refitter` is now a stable way to target one file without over-including similarly named files.
- **SourceGenerator dependency boundary:** `src\Refitter.SourceGenerator\Refitter.SourceGenerator.csproj` keeps `OasReader` private to the generator package and hides `Refit` compile assets from consumers (`PrivateAssets="compile"`). Source generator consumers must carry their own explicit `Refit` reference so Refitter does not silently upgrade them to Refit 10.
- **Focused validation that worked reliably:** use a repo-local NuGet cache (`C:\projects\christianhelle\refitter\.nuget\packages`) when the shared global cache is locked, then run targeted TUnit treenode filters from `src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe` for fast regression checks.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
14 changes: 14 additions & 0 deletions .squad/agents/lambert/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
- **Issue #998 findings (2026-04-16):** Reproduced on clean .NET 10 build. Output.cs written to project root instead of Generated folder. Settings honored, but file path logic broken. Non-default folders work. First build fails due to sync mismatch; second build succeeds. Specific to default single-file output path behavior.
- **PR #1064 closure audit (2026-04-20):** Validation evidence is strong for most closed issues, but #1014, #1040, #1053, and #1055 are over-claimed closures: tests only prove a subset or the code still leaves the reported gap. Manual repros did confirm #1011 (duplicate .refitter filenames now generate distinct hint names), #1012 (MSBuild build now fails on CLI error), and #1031 (settings-relative spec paths validate/generate from repo root).
- **PR #1064 blocker recheck (2026-04-20):** Narrowed repros changed the confidence split: #1021 and #1050 are now proven end-to-end, but #1013 and #1018 are still only partial closures because uncovered collision cases remain reproducible despite the new tests.
- **PR #1067 coverage pass (2026-04-21):** Added direct branch coverage for AOT serializer-context generation (whitespace contracts, OpenAPI-title naming, open-generic rejection, qualified/alias-qualified generic formatting). Swagger 2.0 #1026 compatibility is now explicitly locked for optional `ICollection<T>`, custom reference types, and `IDictionary<string, T>` staying non-nullable while optional value types still fall through as nullable.

### 2026-04-17: Release Compatibility Validation + Tie-Break Repro

Expand Down Expand Up @@ -189,3 +190,16 @@
**Final Session Log:** `.squad/log/2026-04-20T16-00-14Z-pr1064-blocker-fixes.md`

**Merge Status:** ✅ APPROVED (all 12 tests passing; comprehensive edge case coverage)

### 2026-04-20: Remaining Open P1 Worktree Audit

**Task**: Independent tester pass on the still-open P1 fixes under issue #1057 (#1017, #1022, #1023, #1024, #1025, #1026).

**Key findings**:
- `src/Refitter.Core/JsonSerializerContextGenerator.cs` is now wired into `RefitGenerator.Generate()` / `GenerateMultipleFiles()`, but the current implementation uses APIs not available on the `netstandard2.0` target (`ReplaceLineEndings`, range/index syntax, `ToHashSet`), so the worktree does not build yet. This is the current blocker for #1017.
- The new AOT generator also still does not emit any `JsonDerivedType` / polymorphism metadata, and the new JsonSerializerContext test coverage does not exercise polymorphic contracts. Even after the netstandard build break is fixed, #1017 is not fully closed yet.
- `src/Refitter.MSBuild/RefitterGenerateTask.cs` no longer predicts generated files with regex; it now asks the CLI to run in `--simple-output` mode and parses `GeneratedFile:` markers emitted by `src/Refitter/GenerateCommand.cs`. This directly addresses the filename divergence behind #1022 and removes the substring fallback for #1023.
- `src/Refitter.Core/CSharpClientGeneratorFactory.cs` removes the forced `GenerateOptionalPropertiesAsNullable = true` behavior, and `src/Refitter.Tests/Examples/RuntimeCompatibilityTests.cs` now expects nullable-reference-types alone to preserve non-null optional properties. The code change for #1026 looks correct, but it could not be executed end-to-end because #1017 currently breaks the build.
- `src/Refitter.SourceGenerator/Refitter.SourceGenerator.csproj` and `src/Refitter.SourceGenerator/obj/Release/Refitter.SourceGenerator.1.0.0.nuspec` still expose `Refit 10.1.6` and `OasReader 3.5.0.19` as package dependencies, so #1024 remains open.
- `docs/docfx_project/articles/breaking-changes-v2-0-0.md` already documents the Microsoft.OpenApi 1.x → 3.x parser migration, but there is still no comparative smoke-test corpus proving real-world diff coverage, so #1025 remains only partially addressed.
- Temporary repro artifacts are still sitting untracked in the repo root (`.refitter`, `aot-repro.cs`, `aot-repro.json`); they should be cleaned before merge unless intentionally kept.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
28 changes: 28 additions & 0 deletions .squad/agents/parker/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,34 @@
## Learnings

- Team initialized on 2026-04-16.
- 2026-04-20: `JsonSerializerContextGenerator` must use Roslyn syntax analysis instead of regex, emit attributes inside the contracts namespace, register nested types plus closed generic usages, and strip a conventional leading `I` from serializer-context names.
- 2026-04-20: `GenerateJsonSerializerContext` is wired through both `RefitGenerator.Generate()` and `GenerateMultipleFiles()`; multi-file generation emits a dedicated `{ContextName}.cs` file alongside contracts.
- 2026-04-20: `GenerateNullableReferenceTypes` must not silently flip `GenerateOptionalPropertiesAsNullable`; optional-property nullability stays an explicit user choice in `CodeGeneratorSettings`.

## 2026-04-20: P1 Audit Closures (#1017, #1026)

**Task:** Close the remaining core P1 audit findings for AOT serializer-context generation and silent nullable-shape mutation.
**Status:** ✅ COMPLETE — 3 commits created, manual generator/CLI validation passed, focused regression tests added.

**Implementation Summary:**
- **#1017:** Replaced regex-based serializer-context discovery with Roslyn syntax traversal in `src/Refitter.Core/JsonSerializerContextGenerator.cs`, added namespace-safe emission in `RefitGenerator`, and covered nested types, closed generics, multi-file output, and contracts-namespace separation in tests.
- **#1026:** Removed the automatic `GenerateOptionalPropertiesAsNullable` mutation from `src/Refitter.Core/CSharpClientGeneratorFactory.cs`; runtime regressions now assert that NRT alone preserves the old DTO shape while explicit opt-in still yields nullable optional properties.
- **Follow-up hardening:** Added fallback handling for null/blank `Naming.InterfaceName` when deriving serializer-context names and restored a missing Roslyn import in `RefitGenerator.cs` after review.

**Validation Notes:**
- Built `src\Refitter.Core\Refitter.Core.csproj` and `src\Refitter\Refitter.csproj` successfully with a fresh local NuGet cache after shared-cache file locks blocked the default/global cache.
- Manual validation harness confirmed:
- direct serializer-context generation handles nested types and closed generics,
- single-file and multi-file AOT output compile in standalone projects,
- NRT-only generation keeps optional properties non-nullable unless `GenerateOptionalPropertiesAsNullable` is explicitly set.

**Key File Paths:**
- `src/Refitter.Core/JsonSerializerContextGenerator.cs`
- `src/Refitter.Core/CSharpClientGeneratorFactory.cs`
- `src/Refitter.Core/RefitGenerator.cs`
- `src/Refitter.Tests/JsonSerializerContextGeneratorTests.cs`
- `src/Refitter.Tests/Examples/GenerateJsonSerializerContextTests.cs`
- `src/Refitter.Tests/Examples/RuntimeCompatibilityTests.cs`

## 2026-04-20 Final Update: Blockers Implemented and Validated

Expand Down
4 changes: 4 additions & 0 deletions .squad/agents/ripley/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
- Key compatibility surfaces: `.refitter` JSON schema, CLI option names, generated code shape, source generator behavior, MSBuild task predicted paths.
- OasReader jumped from v1.x to v3.x — a major dependency version change that affects OpenAPI parsing.
- Version is already bumped to 1.8.0 in commit `983ad149`.
- `Refitter.MSBuild` now resolves generated files from CLI-emitted `GeneratedFile:` markers in simple-output mode rather than predicting output paths; key files are `src/Refitter.MSBuild/RefitterGenerateTask.cs` and `src/Refitter/GenerateCommand.cs`.
- Safe SourceGenerator packaging uses `OasReader` with `PrivateAssets="all"` and `Refit` with `PrivateAssets="compile"`, plus consumer-facing guidance in `README.md` and verification in `src/Refitter.Tests/SourceGeneratorPackageReferenceTests.cs`.
- The current P1 merge gate is: approve `#1022`, `#1023`, `#1024`; hold `#1017` and `#1025` for more evidence; reject `#1026` until the Swagger 2 nullable-shape regression in `src/Refitter.Tests/Examples/RuntimeCompatibilityTests.cs` is fixed. Temporary repro artifacts (`.refitter`, `aot-repro.*`, `validation-work/`) must be removed before merge.
- Reviewer lockout note: Parker owned the current #1026 implementation slice (`.squad/agents/parker/history.md` runtime/compatibility workstream), so Parker is locked out from the revision after rejection; Ash is the correct revision owner.

### 2026-04-17: Release Compatibility Audit Completion

Expand Down
2 changes: 1 addition & 1 deletion .squad/identity/now.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Current Focus

Fixing the remaining PR #1064 merge blockers: #1013, #1018, and #1053, with targeted regression coverage and a safety gate before merge.
Fixing the remaining open high-severity audit findings linked from issue #1057, with parallel ownership across core, tooling, docs, and an explicit review gate before merge.

Requested by Christian Helle.
29 changes: 29 additions & 0 deletions .squad/skills/analyzer-package-audit/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
---
name: "analyzer-package-audit"
description: "Verify analyzer/source-generator package dependency behavior from the packed output, not just project references"
domain: "packaging"
confidence: "high"
source: "earned"
---

## Context
Use this when auditing NuGet analyzer or source-generator packages for dependency leakage, consumer upgrades, or misleading documentation. `.csproj` metadata alone is not enough because packing can rewrite the dependency surface and bundle assemblies directly into `analyzers/dotnet/cs/`.

## Patterns
- Inspect the generated `.nuspec` (or the `.nupkg` copy of it) to see the actual transitive dependency contract exposed to consumers.
- Inspect the `.nupkg` contents to distinguish:
- **transitive package dependencies** (shown in the nuspec)
- **bundled analyzer-time assemblies** (files under `analyzers/dotnet/cs/`)
- Compare the package's public README with the packed behavior. Inconsistent docs are a false-closure risk even when the code changed.
- For Refitter specifically:
- `Refitter.SourceGenerator` currently bundles `OasReader.dll` as an analyzer asset
- but still publishes `Refit` as a nuspec dependency, so consumers still get Refit transitively

## Examples
- `src/Refitter.SourceGenerator/obj/Release/Refitter.SourceGenerator.1.0.0.nuspec`
- `src/Refitter.SourceGenerator/bin/Release/Refitter.SourceGenerator.1.0.0.nupkg`
- `src/Refitter.SourceGenerator/README.md`

## Anti-Patterns
- Do **not** conclude "dependency leak fixed" solely because a `<PackageReference>` gained `PrivateAssets` in source or because one dependency disappeared from the nuspec.
- Do **not** ignore the packaged README; for NuGet users, that README is part of the shipped behavior.
Loading
Loading