Replace OptionsService with typed settings and ToolConfiguration

[rdeago]

Summary

• Removes OptionsService (a string-keyed bag that laundered typed Spectre settings through magic strings) and SettingsApplier (its populate-from-settings glue).
• Introduces BuildSettingsHolder — a typed DI singleton populated by each command — and Resolve*() methods on BuildSettings/ReleaseSettings so consumers read typed values directly. Renaming a setting now breaks the build at the consumer instead of silently falling back to a default.
• Introduces ToolConfiguration (record) + NuGetPushTarget with a FromEnvironment() static factory for the env-var-only secrets and endpoints (GITHUB_TOKEN, the three *_NUGET_SOURCE/KEY pairs). Same env vars, typed access, registered as a lazy DI singleton. Seedling of the planned file-backed configuration layer — only the factory call site changes when that lands.
• Lifts the verbosity/color wiring into BaseSettings.Apply(IServiceProvider) (partial).
• Closes [bv] Retire env-var-as-option fallback in OptionsService #263: env-var fallback for CLI-flag options is retired in the same PR. The CLI-counterpart env vars (CONFIGURATION, MAIN_BRANCH, the five renamed bv release ones) are no longer recognized; secrets/endpoints are unaffected.
• Changes the set of accepted values for the --verbosity parameter to those accepted by the .NET CLI: quiet/q, minimal/m, normal/n, detailed/d, diagnostic/diag.

Closes #263.

Test plan

• dotnet bv pack succeeds with zero errors and zero warnings; both .nupkg artifacts produced.
• dotnet dnx JetBrains.ReSharper.GlobalTools inspectcode --swea --severity=WARNING … reports zero results.
• CI pipeline green on the PR branch.

[rdeago]

@claude please review this PR.

[claude]

Claude finished @rdeago's task in 4m 47s —— View job

---

PR Review: Replace OptionsService with typed settings and ToolConfiguration

Tasks:

• Gather context and understand request
• Read changed files
• Review CHANGELOG.md
• Review new/added files
• Review modified files
• Review deleted files (OptionsService, SettingsApplier)
• Post review findings

---

Overall Assessment

The refactor is well-structured and a clear improvement: replacing the stringly-typed OptionsService bag with Resolve*() methods on typed settings classes eliminates entire categories of silent-fallback bugs. The ToolConfiguration record is a clean seedling for the future file-backed config layer.

One correctness issue stands out — a regression for non-release CI builds on GitHub Actions. See below.

---

Bug: ToolConfiguration.FromEnvironment() reads all secrets eagerly, breaking non-release builds on GitHub Actions

Severity: High

Trace (new code, any command other than bv clean on GitHub Actions):

1. bv restore / bv build / bv test / bv pack calls BuildSteps.RestoreAsync / BuildAsync / etc.
2. BuildSteps.*Async resolves DotNetService for the first time.
3. DotNetService constructor injects ServerAdapter.
4. ServerAdapter.Create(services) runs; GITHUB_ACTIONS=true → new GitHubServerAdapter(services).
5. GitHubServerAdapter constructor (line 43): _token = services.GetRequiredService<ToolConfiguration>().GitHubToken;
6. This triggers ToolConfiguration.FromEnvironment(), which eagerly calls RequireEnv on all seven env vars: GITHUB_TOKEN, PRIVATE_NUGET_SOURCE, PRIVATE_NUGET_KEY, PRERELEASE_NUGET_SOURCE, PRERELEASE_NUGET_KEY, RELEASE_NUGET_SOURCE, RELEASE_NUGET_KEY.
7. NuGet secrets are typically not present in non-release CI jobs → BuildFailedException.

Before this PR: GitHubServerAdapter only read GITHUB_TOKEN via OptionsService.GetOptionOrFail("githubToken"). NuGet credentials were deferred until NuGetPushAllAsync() (release-only).

Fix options (pick one):

Option A — don't resolve ToolConfiguration at GitHubServerAdapter construction time. Store _services and resolve lazily when the token is actually needed:

// GitHubServerAdapter.cs — constructor
// Remove: _token = services.GetRequiredService<ToolConfiguration>().GitHubToken;
// Change field from `private readonly string _token;` to hold services or use a property

private string Token => _services.GetRequiredService<ToolConfiguration>().GitHubToken;

ToolConfiguration would then only be resolved when PushPassword / CreateGitHubClient() are called — both of which only happen in ReleaseCommand.

Option B — read only GITHUB_TOKEN directly in the constructor, bypassing FromEnvironment():

_token = ToolConfiguration.RequireEnv("GITHUB_TOKEN");

RequireEnv is already internal, so this compiles. NuGet creds would continue to be read lazily in NuGetPushAllAsync().

---

Design observation: implicit construction-time ordering for BuildSettingsHolder

Location: DotNetService.cs:69, GitService.cs:45

Both singletons capture buildSettings.Current in their constructors. The contract is that commands set Current before resolving DotNetService/GitService, which all current commands do correctly. The BuildSettingsHolder XML comment documents this.

The fragile point: if a future step is added to CleanCommand that needs DotNetService, it would silently get Configuration = "Release" (the default-constructed value) rather than failing loudly. Consider making the holder throw if Current is read before it has been explicitly set:

private BuildSettings? _current;
public BuildSettings Current
{
    get => _current ?? throw new InvalidOperationException("BuildSettingsHolder.Current was read before it was set.");
    set => _current = value;
}

Not a bug today, but a low-cost guard against a class of future bugs.

---

Minor: verbosity mapping change not documented in changelog

The old ParseVerbosity mapped "QUIET" → LogLevel.Critical. The new one maps "quiet" → LogLevel.Error. This is a correct alignment with .NET CLI semantics (quiet still shows errors), but it's a subtle behaviour change for anyone using --verbosity quiet. The changelog entry mentions Cake values are no longer accepted, but not this shift. Might be worth a single sentence.

---

What's good

• Resolve*() methods on BuildSettings / ReleaseSettings are the right abstraction: defaults are co-located with the option declaration, and a rename breaks at the consumer rather than silently falling back.
• ReleaseSettings.ResolveBump() correctly combines Enum.TryParse + Enum.IsDefined to reject out-of-range numeric strings.
• ToolConfiguration / NuGetPushTarget split is clean; the internal RequireEnv helper shared via ToolConfiguration is intentional and fine for the same-assembly scope.
• CHANGELOG is well-formatted — the old single-line breaking-change entry for the option renaming was correctly expanded to a list, and the new entries for env-var retirement and verbosity change are accurate and helpful for users migrating.
• BaseSettings-Apply.cs partial-class split follows project conventions cleanly.

[rdeago]

Bug: ToolConfiguration.FromEnvironment() reads all secrets eagerly, breaking non-release builds on GitHub Actions

Fixed.

Design observation: implicit construction-time ordering for BuildSettingsHolder

Added guard against uninitialized use.