Handle numeric fields in GitVersion 6.x JSON output (#218) by dennisdoomen · Pull Request #222 · Fallout-build/Fallout

dennisdoomen · 2026-05-27T09:46:37Z

Summary

Fixes #218

GitVersion 6.x changed BuildMetaData, CommitsSinceVersionSource, PreReleaseNumber, and WeightedPreReleaseNumber from quoted JSON strings to bare JSON integers. System.Text.Json throws:

Cannot get the value of a token type 'Number' as a string.

Root Cause

The GitVersion record declares those four fields as string, but the deserializer has no way to coerce a Number token to a C# string without an explicit converter.

Changes

`src/Fallout.Utilities.Text.Json/NumberToStringJsonConverter.cs` (new)

A JsonConverter<string> that accepts both JSON string tokens and JSON number tokens, converting both to string. Compatible with old (string-output) and new (number-output) GitVersion.

`src/Fallout.Common/Tools/GitVersion/GitVersionTasks.cs`

Added [property: JsonConverter(typeof(NumberToStringJsonConverter))] to the four affected positional record parameters: PreReleaseNumber, WeightedPreReleaseNumber, BuildMetaData, CommitsSinceVersionSource.

`tests/Fallout.Common.Tests/GitVersionParseTest.cs` (new)

Three regression tests using the exact JSON payload from the issue:

Verifies no exception is thrown
Verifies numeric fields are mapped to their string values
Verifies other fields (int, nullable int, plain string) are still correctly populated

Non-Breaking

Consumer code continues to see string for all four fields. GitVersion 5.x (which emits quoted strings) continues to work unchanged.

ChrisonSimtian

Took the branch for a spin against current `main` (now includes #224/#225/#226 since you merged it up) — fix works end-to-end. `tests/Fallout.Common.Tests` runs 35/35 passing (7 env-conditional skips) including your three new `GitVersionParseTest` cases.

Approach is good — much nicer than my first instinct, which was to change the field types to `int`. Keeping them `string` via a custom converter means zero downstream API churn for anyone reading `gitVersion.BuildMetaData` etc. Bidirectional compat (accepts both `"7"` and `7`) keeps older GitVersion installs working.

A couple of small things, both optional / non-blocking:

InvariantCulture on the Double.ToString() fallback. In NumberToStringJsonConverter.Read:
```
JsonTokenType.Number => reader.TryGetInt64(out var l) ? l.ToString() : reader.GetDouble().ToString(),
```
Double.ToString() is culture-sensitive — on de-DE/fr-FR etc. it produces "1,5" instead of "1.5". Realistically GitVersion never emits non-integers here (`BuildMetaData` and friends are integer counts), so this is purely defensive — but worth tightening:
```
JsonTokenType.Number => reader.TryGetInt64(out var l)
    ? l.ToString(CultureInfo.InvariantCulture)
    : reader.GetDouble().ToString(CultureInfo.InvariantCulture),
```
(Int64.ToString() is invariant in practice — no decimal — but it's nice to be explicit on both branches.)
CHANGELOG entry under [Unreleased] — 11.0 would be a nice touch — matches the pattern we set on a previous bug fix in #226. Something like:

Fixed Build.GitVersion injection on GitVersion 6.x (closes #218). GitVersion 6.x emits BuildMetaData, CommitsSinceVersionSource, PreReleaseNumber, and WeightedPreReleaseNumber as JSON numbers instead of quoted strings. New NumberToStringJsonConverter in Fallout.Utilities.Text.Json handles both shapes so consumers continue to see string for all four. Backwards-compatible with GitVersion 5.x output.

Tagged the PR with `target/v11` per our newly-documented PR-creation flow (CLAUDE.md > Semver policy) — Dennis you wouldn't have known about it, no action needed on your end.

Happy to merge as-is or after the polish lands. Your call.

Regression introduced by #175. The generated workflow emitted: - uses: actions/checkout@v6 with: ref: ${{ github.head_ref }} …but didn't set `repository:`. Checkout defaulted to `${{ github.repository }}` (this repo) and tried to resolve `fix/whatever` as a local branch — which fails for fork PRs because the source branch only exists on the contributor's fork. Symptom: cross-repo PRs (e.g. Dennis Doomen's #222 from his fork) fail at the `Run actions/checkout@v6` step with: fatal: A branch or tag with the name 'fix/...' could not be found #175's `CheckoutRef = "${{ github.head_ref }}"` was added so GitVersion / GitRepository.FromLocalDirectory see the source branch in `.git/HEAD` (otherwise checkout puts you on the merge ref in detached-HEAD state). Goal correct; cross-repo coverage missed. Fix: - GitHubActionsCheckoutStep.Write now emits a `repository:` line whenever `Ref` is non-empty: repository: ${{ github.event.pull_request.head.repo.full_name || github.repository }} The `||` fallback covers push events (no pull_request context). - Doc comments on CheckoutRef + Ref updated. - New regression test case `checkout-ref` in ConfigurationGenerationTest locks the YAML shape. Verified snapshot committed alongside. - .github/workflows/ubuntu-latest.yml regenerated by the build. macos-latest / windows-latest / release.yml unchanged (none set CheckoutRef). - CHANGELOG.md entry under [Unreleased] — 11.0. After this lands, Dennis's PR #222 will pick up the fix on the next workflow run (any rerun, or a push to his branch). Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@ChrisonSimtian

…/tests (#232) * chore(repo): add CODEOWNERS pointing to @ChrisonSimtian Prep for tightening main branch protection now that external contributors are opening PRs (dennisdoomen, #222 / #230). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * chore(repo): scope CODEOWNERS to /src and /tests only Docs, build orchestrator, CI workflows, and root config remain unowned. The follow-up branch-protection update will pair this with required_approving_review_count=0 + require_code_owner_reviews=true, so docs PRs need only CI green while src/tests PRs require explicit owner approval. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

dennisdoomen · 2026-05-28T09:48:40Z

@ChrisonSimtian Why did you merge main in my branch?

ChrisonSimtian · 2026-05-28T19:51:42Z

@ChrisonSimtian Why did you merge main in my branch?

ah sorry must've been in the wrong tab, that wasn't on purpose 😅

GitVersion 6.x changed BuildMetaData, CommitsSinceVersionSource, PreReleaseNumber and WeightedPreReleaseNumber from quoted strings to bare JSON integers. System.Text.Json cannot coerce a Number token to a C# string without an explicit converter. - Add NumberToStringJsonConverter in Fallout.Utilities.Text.Json that accepts both JSON string and JSON number tokens, returning a string. - Annotate the four affected positional parameters in the GitVersion record with [property: JsonConverter(typeof(NumberToStringJsonConverter))]. - Add GitVersionParseTest with three regression tests using the exact payload from the issue report. Non-breaking: consumer code still sees string for all four fields; GitVersion 5.x (which emits quoted strings) continues to work. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Mourad57 · 2026-07-11T07:50:20Z

Hello Guys,

I do have actually the same error using GitVersion version 5 (5.12.0), did this fix made it in a stable release ?

Thanks

ChrisonSimtian · 2026-07-11T08:54:50Z

not yet but we'll release another stable soon. in the meantime you can use the gh packages as a preview

Mourad57 · 2026-07-11T10:21:25Z

ok great

thx

dennisdoomen marked this pull request as draft May 27, 2026 09:47

ChrisonSimtian self-assigned this May 27, 2026

ChrisonSimtian marked this pull request as ready for review May 27, 2026 10:39

ChrisonSimtian added the target/v11 label May 27, 2026

ChrisonSimtian reviewed May 27, 2026

View reviewed changes

This was referenced May 27, 2026

RFC: where do JSON converters and similar utility primitives belong in the Fallout architecture? #228

Closed

fix(ci): [GitHubActions] CheckoutRef now resolves cross-repo PRs #229

Merged

ChrisonSimtian marked this pull request as draft May 27, 2026 11:09

dennisdoomen changed the title ~~fix: handle numeric fields in GitVersion 6.x JSON output (#218)~~ Handle numeric fields in GitVersion 6.x JSON output (#218) May 27, 2026

dennisdoomen force-pushed the fix/gitversion-numeric-fields-218 branch 6 times, most recently from eedd359 to 9802daf Compare May 27, 2026 17:02

dennisdoomen marked this pull request as ready for review May 27, 2026 17:03

ChrisonSimtian assigned dennisdoomen May 27, 2026

ChrisonSimtian mentioned this pull request May 27, 2026

chore(repo): add scoped CODEOWNERS + tighten main review gate for src/tests #232

Merged

3 tasks

dennisdoomen force-pushed the fix/gitversion-numeric-fields-218 branch from 50909a3 to 32b1b3a Compare May 28, 2026 10:12

ChrisonSimtian mentioned this pull request May 29, 2026

Cut release/v11 branch from current main #269

Closed

3 tasks

dennisdoomen force-pushed the fix/gitversion-numeric-fields-218 branch 2 times, most recently from 55f92b9 to 04dafa8 Compare May 29, 2026 06:08

ChrisonSimtian self-requested a review May 29, 2026 06:27

ChrisonSimtian approved these changes May 29, 2026

View reviewed changes

ChrisonSimtian mentioned this pull request May 29, 2026

chore(tools): complete 10.3 contaminated-version unlist batch (10.3.24-47) #294

Merged

ChrisonSimtian enabled auto-merge (squash) May 29, 2026 08:11

ChrisonSimtian force-pushed the fix/gitversion-numeric-fields-218 branch from 04dafa8 to 65e4f49 Compare May 29, 2026 08:12

ChrisonSimtian force-pushed the fix/gitversion-numeric-fields-218 branch from 65e4f49 to 9234df3 Compare May 29, 2026 09:24

ChrisonSimtian removed the target/v11 label May 29, 2026

ChrisonSimtian added this to the 10.3.0 - Continue Matt's direction milestone May 29, 2026

ChrisonSimtian merged commit d87d6ea into Fallout-build:main May 29, 2026
2 checks passed

ChrisonSimtian added the bug Something isn't working label Jul 11, 2026

dennisdoomen deleted the fix/gitversion-numeric-fields-218 branch July 11, 2026 10:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Handle numeric fields in GitVersion 6.x JSON output (#218)#222

Handle numeric fields in GitVersion 6.x JSON output (#218)#222
ChrisonSimtian merged 1 commit into
Fallout-build:mainfrom
dennisdoomen:fix/gitversion-numeric-fields-218

dennisdoomen commented May 27, 2026 •

edited

Loading

Uh oh!

ChrisonSimtian left a comment

Uh oh!

dennisdoomen commented May 28, 2026

Uh oh!

ChrisonSimtian commented May 28, 2026

Uh oh!

Uh oh!

Mourad57 commented Jul 11, 2026

Uh oh!

ChrisonSimtian commented Jul 11, 2026

Uh oh!

Mourad57 commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Uh oh!

Conversation

dennisdoomen commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Changes

src/Fallout.Utilities.Text.Json/NumberToStringJsonConverter.cs (new)

src/Fallout.Common/Tools/GitVersion/GitVersionTasks.cs

tests/Fallout.Common.Tests/GitVersionParseTest.cs (new)

Non-Breaking

Uh oh!

ChrisonSimtian left a comment

Choose a reason for hiding this comment

Uh oh!

dennisdoomen commented May 28, 2026

Uh oh!

ChrisonSimtian commented May 28, 2026

Uh oh!

Uh oh!

Mourad57 commented Jul 11, 2026

Uh oh!

ChrisonSimtian commented Jul 11, 2026

Uh oh!

Mourad57 commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dennisdoomen commented May 27, 2026 •

edited

Loading

`src/Fallout.Utilities.Text.Json/NumberToStringJsonConverter.cs` (new)

`src/Fallout.Common/Tools/GitVersion/GitVersionTasks.cs`

`tests/Fallout.Common.Tests/GitVersionParseTest.cs` (new)