Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions .squad/agents/ash/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
- In the source generator, treat `AdditionalText.GetText(...)` as nullable and convert read/encoding failures into diagnostics; null-forgiving it weakens failure reporting.
- Review gate for the next safety pass: require targeted tests for source-generator hint-name collisions, OpenAPI-title sanitization including `<`/`>`, and MSBuild runtime-discovery timeout behavior before accepting cleanup claims.
- 2026-04-28: Approved Parker/Lambert e-conomic multi-spec merge fix after verifying canonical JSON-token equivalence accepts duplicate recursive/shared schemas, conflicting duplicate path/schema/definition/security entries still fail fast, and focused net10.0 TUnit coverage passes.
- 2026-04-29T10:41:29.997+02:00: `src\Refitter.Core\OpenApiDocumentFactory.cs` should clone merge baselines with `OpenApiDocument.ToJson()` plus `OpenApiDocument.FromJsonAsync(...)`; the parameterless serializer preserves `SchemaType` for both Swagger 2 and OpenAPI 3 while avoiding obsolete `ToJson(SchemaType)` usage.
- 2026-04-29T10:41:29.997+02:00: The safety proof for this obsolete-API lane lives in `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs`: round-trip embedded Swagger Petstore V2/V3 fixtures, then assert mixed-version merges keep the base document schema type and serialized top-level marker (`swagger` vs `openapi`).
- 2026-04-29T10:41:29.997+02:00: For focused core-lane validation in this repo, `src\Refitter.Tests\bin\Release\net10.0\Refitter.Tests.exe --treenode-filter '/*/Refitter.Tests/OpenApiDocumentFactoryMergeTests/*' --disable-logo --no-progress --no-ansi` reliably runs the merge test class when `dotnet test --filter ...` is unavailable under TUnit.

## Core Context

Expand Down Expand Up @@ -74,3 +77,13 @@
- Ripley's sequencing and Lambert's green baseline keep docs/help drift as the safe first cleanup lane, but Ash's gate still holds the deeper generator and MSBuild cleanup as validation-first work.
- Dallas's docs clarification commit `f6374210` landed cleanly, so the next review concern stays on hint-name uniqueness, null-safe source-generator input handling, identifier sanitization, and MSBuild runtime-discovery timeout coverage.

## 2026-04-29: OpenApiDocument Clone Path Obsolete API Approval

- **2026-04-29T08:41:29Z Final Safety Approval:** Confirmed `src\Refitter.Core\OpenApiDocumentFactory.cs` clone path is safe to migrate from obsolete `ToJson(SchemaType)` to parameterless `ToJson()` + `FromJsonAsync()` round-trip.
- **Evidence Verified:**
- Schema-type preservation across round-trip serialization for Swagger 2 and OpenAPI 3
- Merge contract integrity (clone-first, fail-fast on genuine conflicts)
- Focused `OpenApiDocumentFactoryMergeTests` covering embedded Swagger Petstore V2/V3 fixtures
- Mixed-version merge assertions confirming correct schema type and top-level markers
- **Validation Results:** Release build ✓, full Refitter.Tests ✓, Refitter.SourceGenerator.Tests ✓, format verification ✓
- **Outcome:** APPROVED for production fix. Obsolete API successfully removed from merge-critical clone path while maintaining all semantic guarantees.
8 changes: 8 additions & 0 deletions .squad/agents/lambert/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -87,3 +87,11 @@
- 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.
8 changes: 8 additions & 0 deletions .squad/agents/parker/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,11 @@
- Test coverage: merge equivalence tests, e-conomic-shaped regressions, compile-backed generation validation
- Constraints: preserve clone-first/non-mutating merge, don't rename schemas, keep relative-path behavior across tooling surfaces
- 2026-04-28T15:21:48.369+02:00: Implemented duplicate schema equivalence by comparing normalized NSwag/NJsonSchema JSON tokens, with a schema-specific canonical fallback for referenced/cyclic graphs that cannot serialize standalone. This keeps merge semantics fail-fast for real path/schema/definition/security conflicts while allowing equivalent e-conomic `ProblemDetails`/`Error` duplicates.

## 2026-04-29: OpenApiDocument Clone Path Obsolete API Fix

- 2026-04-29T08:41:29Z: Replaced obsolete `OpenApiDocument.ToJson(SchemaType)` clone logic in `src\Refitter.Core\OpenApiDocumentFactory.cs` with parameterless `ToJson()` round-trip via `FromJsonAsync()`.
- Preserves `SchemaType` across serialization round-trip for both Swagger 2 and OpenAPI 3 documents.
- Maintains clone-first/fail-fast merge contract without carrying obsolete API forward.
- Validation: release build ✓, full test suite ✓, format verification ✓
- Outcome: Approved by Ash and Lambert after focused regression coverage confirmed schema-type preservation.
10 changes: 10 additions & 0 deletions .squad/agents/ripley/history.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
- Team initialized on 2026-04-16.
- 2026-04-28T12:02:17.298+02:00: Issue #1045's new multi-path `.refitter` comment does not reproduce at current HEAD; current CLI validation accepts `openApiPaths`, normalizes relative entries, and the live repro instead surfaced expected merge-collision behavior only when duplicate path specs were used.
- 2026-04-28T15:21:48.369+02:00: The e-conomic multi-spec failure is a core `OpenApiDocumentFactory` merge-equivalence problem: `economic-products.json` and `economic-webhooks.json` share equivalent `Error` and `ProblemDetails` component schemas, but current NSwag-object serialization comparison false-negatives and throws before validation/generation.
- 2026-04-29T11:51:36.530+02:00: PR assembly for `build-warnings` should target `main` and be framed as a branch-wide warning-hardening pass, not only the latest `OpenApiDocumentFactory` change; the landed scope spans `src\Refitter.Core\OpenApiDocumentFactory.cs`, `src\Refitter.Core\Refitter.Core.csproj`, `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs`, `src\Refitter.Tests\Refitter.Tests.csproj`, `src\Refitter\Refitter.csproj`, and the carried `.squad` history/decision sync.

## Core Context

Expand Down Expand Up @@ -40,3 +41,12 @@
- Fix approved: replace `AreEquivalent()` with OpenAPI-aware semantic comparison using NSwag canonical JSON representation; keep fail-fast for genuine conflicts.
- Gates: merge equivalence tests, e-conomic-shaped regressions, compile-backed validation; existing collision tests must pass.
- Constraints: preserve relative-path behavior, don't edit specs, don't rename schemas in merge.

## 2026-04-29: PR Assembly for Build-Warnings

- **PR #1079**: "Tighten warning handling across Refitter builds"
- **Target branch**: main (default branch for this repo)
- **Scope framing**: Branch includes multiple warning-focused commits across core, tests, CLI packaging metadata, nullable-flow cleanup, and warning enforcement; obsolete `OpenApiDocument.ToJson(SchemaType)` replaced with `ToJson()` + `FromJsonAsync()`.
- **Key files**: `src\Refitter.Core\OpenApiDocumentFactory.cs`, `src\Refitter.Tests\OpenApiDocumentFactoryMergeTests.cs`, `src\Refitter.Core\Refitter.Core.csproj`, `src\Refitter.Tests\Refitter.Tests.csproj`, `src\Refitter\Refitter.csproj`, `.squad` sync.
- **PR assembly decision recorded** to decisions.md (2026-04-29).
- **Branch pushed** with squad metadata committed.
121 changes: 121 additions & 0 deletions .squad/decisions.md
Original file line number Diff line number Diff line change
Expand Up @@ -350,3 +350,124 @@ Rationale:
- 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 #<new-issue-number>` 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.
25 changes: 25 additions & 0 deletions .squad/skills/pr-scope-framing/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
---
name: "pr-scope-framing"
description: "Assemble honest PR titles/bodies when a branch contains multiple landed commit lanes"
domain: "github, review"
confidence: "high"
source: "earned"
last_updated: "2026-04-29T11:51:36.530+02:00"
---

## Context

Use this when opening a PR from a branch that has accumulated several already-committed changes, especially if the most recent commit is narrower than the full diff reviewers will receive.

## Pattern

- Check whether a PR already exists for the branch before drafting new wording.
- Summarize the full branch diff against the target base branch, not just `HEAD~1..HEAD`.
- Choose a title/body that matches the branch's aggregate scope and call out any especially important fix as a bullet, not the whole frame, unless it truly dominates the diff.
- If branch metadata or team-history commits are already present, disclose them briefly so reviewers are not surprised by non-product files in the PR.

## Anti-patterns

- Titling a PR after only the latest fix when the branch contains multiple unrelated or semi-related commits.
- Hiding already-committed coordination files that will obviously appear in review.
- Reusing stale issue-closure language without re-checking the actual branch contents and current PR state.
1 change: 1 addition & 0 deletions .squad/skills/tunit-test-filtering/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ Refitter test projects use TUnit on Microsoft.Testing.Platform. In this repo, `d
- `/*/Refitter.Tests/XmlDocumentationGeneratorTests/Can_Generate_Method_Throws_With_Readable_Unicode_Status_Code_Comments`
- `/*/Refitter.Tests.Examples/GenerateStatusCodeCommentsTests/Generated_Code_Preserves_Readable_Unicode_In_Status_Code_Comments`
- `/*/Refitter.Tests.Examples/GenerateStatusCodeCommentsTests/Generated_Code_With_Unicode_Status_Code_Comments_Can_Build`
- `/*/Refitter.Tests/OpenApiDocumentFactoryMergeTests/*` runs the whole merge test class, which is useful for clone/merge safety checks around `OpenApiDocumentFactory`.

## Anti-Patterns
- Do **not** rely on `dotnet test --filter ...` here; the TUnit/Microsoft.Testing.Platform test app rejects that option.
Expand Down
5 changes: 4 additions & 1 deletion src/Refitter.Core/OpenApiDocumentFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ public static async Task<OpenApiDocument> CreateAsync(IEnumerable<string> openAp

private static OpenApiDocument Merge(OpenApiDocument[] documents)
{
var baseDocument = OpenApiDocument.FromJsonAsync(documents[0].ToJson(documents[0].SchemaType)).GetAwaiter().GetResult();
var baseDocument = CloneDocument(documents[0]);
var tagNames = new HashSet<string>(baseDocument.Tags.Select(t => t.Name), StringComparer.Ordinal);

for (var i = 1; i < documents.Length; i++)
Expand Down Expand Up @@ -106,6 +106,9 @@ private static OpenApiDocument Merge(OpenApiDocument[] documents)
return baseDocument;
}

private static OpenApiDocument CloneDocument(OpenApiDocument document)
=> OpenApiDocument.FromJsonAsync(document.ToJson()).GetAwaiter().GetResult();

private static void MergeIfMissingOrThrowOnConflict<TValue>(
IDictionary<string, TValue> target,
string key,
Expand Down
10 changes: 9 additions & 1 deletion src/Refitter.Core/RefitGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,15 @@ private static async Task<OpenApiDocument> GetOpenApiDocument(RefitGeneratorSett
{
if (settings.OpenApiPaths is { Length: > 0 })
return await OpenApiDocumentFactory.CreateAsync(settings.OpenApiPaths).ConfigureAwait(false);
return await OpenApiDocumentFactory.CreateAsync(settings.OpenApiPath).ConfigureAwait(false);

if (string.IsNullOrWhiteSpace(settings.OpenApiPath))
{
throw new ArgumentException(
"Either OpenApiPath or OpenApiPaths must be provided with at least one valid OpenAPI specification path.",
nameof(settings));
}

return await OpenApiDocumentFactory.CreateAsync(settings.OpenApiPath!).ConfigureAwait(false);
}

private static void ProcessContractFilter(OpenApiDocument openApiDocument, bool removeUnusedSchema, string[] includeSchemaMatches,
Expand Down
1 change: 1 addition & 0 deletions src/Refitter.Core/Refitter.Core.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<RootNamespace>Refitter.Core</RootNamespace>
<Product>Refitter Core</Product>
<Description>Core library used by the Refitter CLI tool for generating a C# REST API Client using the Refit library.</Description>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>

<ItemGroup>
Expand Down
Loading
Loading