Add buildvana.json configuration foundation

[rdeago]

Summary

Phase 1 of #267 (implements #268): the shared configuration foundation. Introduces Buildvana.Core.Configuration, a host-agnostic Core-tier library (System.Text.Json + BCL only) holding the full typed BuildvanaConfig model and a loader, consumed by bv now and Buildvana.Sdk.Tasks in a later phase. The configuration is inert: discovered, parsed, validated, and registered in the tool's DI, but nothing reads it yet — no behavior changes.

Closes #268.

What's included

• Model + loader — full BuildvanaConfig shape (release, versioning, dotnet, nuget, github, git), everything optional. Discovery is buildvana.json xor buildvana.jsonc in the home directory; parse uses Skip comments, trailing commas, and Disallow unmapped members, with $schema whitelisted. Dictionary sections get explicit key validation (dotnet.args: all/restore/build/test/pack; nuget.feeds: prerelease/release). Failures surface as BuildFailedException.
• Schema — schemas/buildvana.schema.json generated from the model via JsonSchemaExporter (reusing [Description]s for hover docs). tools/generate-config-schema.cs regenerates (--update) / verifies (--check) it; CI fails on drift.
• Home-directory discovery — now stops at the nearest directory (start included) containing any marker: a config file, .buildvana-home, or a Git marker. Behaviour mirrored between HomeDirectoryDiscovery and Sdk.props; new BVSDK1005 when both config variants are present.
• Tool DI — BuildvanaConfig registered as a singleton (inert).
• CHANGELOG entry under Unreleased.

Acceptance criteria

• Library builds, referenced by Buildvana.Tool; absent file ⇒ defaults, no behavior change.
• Malformed file / unknown key / unknown dict key / both variants present each fail with a clear message.
• Committed schema matches the model (CI staleness check); editors validate against it (draft 2020-12 dialect declared).
• Discovery recognizes buildvana.json/.jsonc and still recognizes .buildvana-home/.git*, in both tool and SDK.
• CHANGELOG entry for the (inert) configuration file.

Notes / decisions

• Full inert model now (all sections defined); Phases 2–3 wire values into behavior.
• Staleness check is a CI-run single-file tool under tools/, not a unit-test project (the repo has none yet).
• nuget.feeds keys are prerelease/release only (private dropped from the older spec).
• Drive-by: corrected a stale BVESDK003 → BVSDK1003 in docs/Diagnostics.md.
• DTO records carry [UsedImplicitly(…WithMembers)] so ReSharper doesn't flag serializer-only init accessors.

Test plan

• dotnet bv pack clean (both packages produced).
• ReSharper inspectcode --severity=WARNING — 0 results.
• Loader verified across 11 cases; C#↔MSBuild discovery verified equivalent across config-vs-git, git-fallback, and both-files scenarios.

🤖 Generated with Claude Code

[rdeago]

@claude please review this PR.

[claude]

Claude finished @rdeago's task in 6m 5s —— View job

---

PR Review

• Gather context and understand request
• Read new Buildvana.Core.Configuration library files
• Read modified home directory discovery files
• Read modified SDK props and tool files
• Read schema and schema generation tool
• Read CI workflow changes
• Compile and post review findings

---

Overall this is a clean, well-structured implementation. The design decisions are sound — separating model from loader, sharing serialization options between the loader and schema generator so they stay in sync, the CI staleness check. Below are the findings, roughly in priority order.

---

🟡 Schema/loader divergence on constrained dictionary keys

File: schemas/buildvana.schema.json lines 130–148 (dotnet.args) and 159–190 (nuget.feeds)

The JSON schema uses additionalProperties (open-ended), but the loader only accepts specific keys: all/restore/build/test/pack for dotnet.args and prerelease/release for nuget.feeds. An editor will happily validate { "dotnet": { "args": { "compile": [...] } } } as conformant, then the loader rejects it at runtime with no prior warning in the IDE.

This is a DX gap — the two layers disagree. The root cause is that JsonSchemaExporter doesn't know about the application-level key restriction.

Fixing it requires a post-generation patch in BuildvanaConfigSchema.Generate(), after the exporter runs, to replace the open additionalProperties with an explicit properties + additionalProperties: false block for these two dictionaries. It's a few lines of JsonObject manipulation in BuildvanaConfigSchema.cs. Not a blocker for Phase 1 (config is inert), but worth noting before Phase 2 ships.

Fix this →

---

🟢 Schema permits null where the loader rejects it

File: schemas/buildvana.schema.json line 4–7

"type": ["object", "null"]

The root type includes null, so buildvana.json containing just null passes schema validation. The loader explicitly rejects it:

// BuildvanaConfigLoader.cs:73
return config ?? throw new BuildFailedException($"{path} was parsed as JSON null.");

Minor inconsistency — the schema advertises a superset of what the loader accepts. Since BuildvanaConfig is a C# reference type the exporter correctly reflects its nullability, but a non-nullable wrapper type or a custom post-generation patch dropping null from the root type would close the gap.

---

🟢 Schema allows null items in array properties

File: schemas/buildvana.schema.json lines 29–34

"items": { "type": ["string", "null"] }

release.branches (and release.generateDocsFrom) items are typed ["string", "null"], which permits ["main", null]. The loader does not validate list items for nullness. When these arrays are actually consumed in Phase 2, a null item would either be silently ignored or throw a NullReferenceException depending on the call site. Worth having validation in the loader when Phase 2 wires these up (or a non-nullable item type annotation that the schema exporter can pick up).

---

🟢 DI registration is lazy — Phase 1 does not actually validate the config

File: src/Buildvana.Tool/Program.cs line 73

.AddSingleton(static sp => BuildvanaConfigLoader.Load(sp.GetRequiredService<IHomeDirectoryProvider>().HomeDirectory))

Microsoft.Extensions.DependencyInjection singletons registered with a factory are initialized on first resolve. Since no command in Phase 1 resolves BuildvanaConfig, the factory never runs — meaning a malformed buildvana.json goes silently undetected until Phase 2. The PR description says "discovered, parsed, validated", but the implementation is lazy. Not a correctness bug (it's consistent with "inert"), just worth adjusting the description or adding a comment here so the intent is clear.

---

🟢 CountLines off-by-one (cosmetic)

File: tools/generate-config-schema.cs line 111

static int CountLines(string text) => text.Split('\n').Length;

For any text ending in \n (which BuildvanaConfigSchema.Generate() guarantees), Split produces a trailing empty element, so the reported count is one higher than the actual line count (e.g., "246 lines" for a 245-line file). Display only, but easy to fix: text.Split('\n').Length - 1 or text.Count(c => c == '\n').

---

Positive notes

• Home-directory discovery alignment: The old C# algorithm used marker priority (.buildvana-home > .git > .git/HEAD) while the old MSBuild SDK already used proximity priority (longest path wins). The new C# code correctly aligns with MSBuild — both now return the nearest directory containing any marker. The behavioral change is intentional and well-documented.

• BVSDK1005 and the BVESDK003 → BVSDK1003 correction: Both documented correctly in docs/Diagnostics.md. The SDK-side duplicate-config check and the tool-side check in BuildvanaConfigLoader are symmetric and consistent.

• Shared BuildvanaConfigSerialization.Options: Using the same JsonSerializerOptions instance for both deserialization and schema generation is the right call — it guarantees the committed schema reflects exactly what the loader accepts (enum casing, camelCase naming, etc.).

• CI staleness check: dotnet run tools/generate-config-schema.cs -- --check before the build is a clean solution for keeping the schema in sync without a full test project.

• Error messages: All failure paths produce actionable messages (what was found, what was expected, where to look). The ValidateDictionaryKeys message listing the allowed values is especially good.

• $schema whitelisting: Clean approach — model the property explicitly, let UnmappedMemberHandling.Disallow handle everything else.

[rdeago]

@claude thanks for the review — all five findings addressed, one commit each:

• 🟡 Schema/loader divergence on dict keys (135adf5): the generator now patches dotnet.args and nuget.feeds to explicit properties + additionalProperties: false. The allowed-key lists moved onto DotNetConfig/NuGetConfig as a single ordered source shared by the loader and the schema generator, so the two can't drift apart.
• Schema permits null at root (120e3c3): root type forced to "object" after export.
• Schema allows null array items (fbc9959): array item types tightened to non-null during generation. To avoid creating the opposite divergence, the loader now also rejects null items in release.branches, release.generateDocsFrom, and dotnet.args value lists.
• Lazy DI registration (56952f2): added a comment at the registration site spelling out that discovery/parse/validation run on first resolve and stay inert until a Phase 2 consumer reads the config.
• CountLines off-by-one (6b42f27): now Split('\n').Length - 1.

Verified with dotnet bv pack (both packages produced), ReSharper inspectcode --severity=WARNING (0 results), and the schema staleness check.

@claude please review again.

[claude]

Claude finished @rdeago's task in 2m 31s —— View job

---

PR Re-Review

• Gather context and understand request
• Read fixes for schema/loader dict key divergence (135adf5)
• Read fix for root type null (120e3c3)
• Read fix for null array items + loader validation (fbc9959)
• Read lazy DI comment (56952f2)
• Read CountLines fix (6b42f27)
• Compile and post re-review findings

---

All five findings have been addressed cleanly. Here's a pass on each fix and a note on one new observation.

---

🟡 → ✅ Schema/loader dict key divergence (135adf5)

ConstrainKeys in BuildvanaConfigSchema.cs correctly:

1. Captures the additionalProperties value schema before removing it
2. Builds an explicit properties object with DeepClone() for each allowed key
3. Sets additionalProperties: false

The AllowedArgsKeys / AllowedFeedKeys static arrays in DotNetConfig / NuGetConfig are now the single source of truth shared by both Validate (loader) and ConstrainKeys (schema generator). The schema output confirms the expected shape (dotnet.args with all, restore, build, test, pack; nuget.feeds with prerelease, release; both sealed with additionalProperties: false).

One minor observation (not a bug): StripNullFromArrayItems first calls RemoveNullFromType(items) directly, then the foreach over the object's properties later recurses back into the same items node. The second pass is a no-op (the type is already a scalar by then), so it's harmless — just slightly redundant. Not worth changing.

---

🟢 → ✅ Root type null (120e3c3)

root["type"] = "object" is the correct fix. Schema line 4 confirms "type": "object" (no null).

---

🟢 → ✅ Null array items (fbc9959)

StripNullFromArrayItems walks the full schema tree and RemoveNullFromType collapses ["string", "null"] → "string". Schema output confirms all items are "type": "string" (e.g., release.branches, release.generateDocsFrom, dotnet.args.{key}).

The loader's ValidateNoNullItems now guards release.branches, release.generateDocsFrom, and each dotnet.args.{key} value list. The model types (IReadOnlyList<string>) are non-nullable element types, so the validation mirrors the model exactly.

Note: ChangelogUpdates enum still includes null in the schema — correct, since that represents the absent optional field, not a list item.

---

🟢 → ✅ Lazy DI comment (56952f2)

Program.cs lines 74–77 now carry a comment explaining that the factory runs on first resolve, that no Phase 1 command resolves BuildvanaConfig, and that a malformed file stays inert until Phase 2. This is exactly the right place and right framing.

---

🟢 → ✅ CountLines off-by-one (6b42f27)

text.Split('\n').Length - 1 is correct. The new comment on line 111 ("Generate() guarantees a trailing newline, so the line count is the number of line terminators") explains the invariant cleanly.

---

Summary

All five findings are fixed correctly with no new issues introduced. The single-source-of-truth approach for the allowed key lists is the right design — the loader and schema generator are now structurally coupled and cannot drift. The PR is ready to merge.