Move secrets/endpoints into buildvana.json (Config Phase 3)

[rdeago]

Closes #270 (Config Phase 3 of #267).

bv's NuGet push feeds and GitHub token now come from buildvana.json instead of the seedling *.FromEnvironment types. The nuget.feeds, github.tokenEnv, and git.identity model + schema shapes already shipped in the Phase 1 foundation, so this PR is the wiring plus removal of the seedlings.

Changes

• DotNetSettings owns NuGet push resolution (the push is a dotnet nuget push invocation). The six DotNetInvocationSettings move under a new Invocations sub-object (DotNetInvocationsSettings), and ResolvePushTarget(isPrerelease) routes prerelease → release, with prerelease falling back to release when absent. The feed source comes from config; the API key is read from the env var named by apiKeyEnv at point of use. Missing feed/source/apiKeyEnv fail with a clear, channel-named message.
• GitHubServerAdapter resolves github.tokenEnv (default GITHUB_TOKEN) and reads it via EnvVarHelper.Require.
• The private feed channel is dropped. IsPrivateRepositoryAsync() still compiles and is callable but no longer selects a feed.
• Deleted ToolConfiguration and NuGetPushConfiguration; NuGetPushTarget is now a plain resolved-value record.
• CHANGELOG: New-features entries for the config-driven secrets/endpoints and git.identity; corrected the now-stale "secrets… unaffected" note.

Acceptance criteria

• Push targets and the GitHub token resolve from buildvana.json (apiKeyEnv/tokenEnv read at point of use); the *.FromEnvironment paths are gone.
• With only nuget.feeds.release set, prereleases publish to the release feed (covered by a unit test).
• IsPrivateRepositoryAsync() still compiles and is callable but no longer selects a feed.
• CHANGELOG updated.

Verification

• dotnet bv pack — clean, artifacts produced.
• ReSharper inspectcode --severity=WARNING — 0 results (also cleaned 2 pre-existing redundant-using warnings in DotNetServiceMergeInvocationTests).
• Build 0 warnings / 0 errors; 97/97 tests pass (7 new).

[rdeago]

@claude please review this PR.

[claude]

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

---

PR Review

• Gather context and understand request
• Read changed files
• Analyze code for correctness, style, and best practices
• Post review findings

---

Summary

This is a clean, focused PR that delivers exactly what it advertises: moving secrets/endpoints from *.FromEnvironment seedlings into buildvana.json, with a proper fallback chain and clear error diagnostics. The architecture is sound and the implementation is correct.

---

Observations

DotNetSettings.ResolvePushTarget — feed selection logic

The pattern for prerelease fallback is clear and correct. One subtlety worth noting: ResolvePushTarget is resolved once per push operation (before the package loop in NuGetPushAllAsync, line 249 of DotNetService.cs), which means all packages in a single release go to the same feed. That's exactly right, and also means the API key env var doesn't need to exist when there are no packages to push (early return at line 246).

Token eager vs. key lazy

The GitHub token (_token) is read at adapter creation time (constructor, GitHubServerAdapter.cs line 47), while NuGet API keys are read at push time (ResolvePushTarget, line 83 of DotNetSettings.cs). The CHANGELOG and PR description describe the NuGet keys as "read at the point of use", which is accurate, but the token is effectively "read at startup". This is a behaviorally intentional asymmetry — the token is needed throughout the adapter's lifetime (for release creation, asset upload, etc.), so eager resolution makes sense and gives earlier diagnostics. Just worth being aware of for future tokenEnv documentation.

Minor test coverage gaps (non-blocking)

Tests cover the release channel for missing source and apiKeyEnv. The symmetric cases for the prerelease channel (prerelease feed present but missing source or apiKeyEnv) are not tested. The code path is symmetric so it would work correctly, but it might be nice to add:

[Test]
public async Task ResolvePushTarget_Throws_WhenPrereleaseFeedHasNoSource()
{
    var config = ConfigWithFeeds(
        ("prerelease", new() { ApiKeyEnv = "BV_TEST_PRERELEASE_API_KEY" }),
        ("release", new() { Source = "https://release.example/v3/index.json", ApiKeyEnv = "BV_TEST_UNUSED" }));
    var settings = new DotNetSettings(config);
    await Assert.That(() => settings.ResolvePushTarget(isPrerelease: true)).Throws<BuildFailedException>();
}

Not a blocker, but a small gap.

DotNetInvocationsSettings fallback

Resolve(config?.NugetPush) correctly uses DotNetInvocationSettings.Empty when the config section is absent — no allocations, no null propagation downstream. Good.

No dead references to deleted types

Confirmed: no remaining references to ToolConfiguration, NuGetPushConfiguration, or NuGetPushTarget.FromEnvironment in the source tree. The DI registrations in Program.cs are cleanly removed.

CHANGELOG

The new entries accurately describe the behavior, including the correct "at the point of use" wording for secrets and the corrected stale note about fixed env vars. Markdown style uses _emphasis_ and **strong** correctly per project conventions.

---

Verdict

LGTM. The implementation is correct, the error messages are clear and channel-named, the test suite covers the main behavioral branches well, and the removal of the seedling types is clean. The minor test gap above is non-blocking.

[rdeago]

Thanks @claude for your review!

Minor test coverage gaps (non-blocking)

I have added separate tests for missing keys in prerelease and release, as you suggested.

No other changes. If you'd like to take another look, be my guest.

[claude]

Claude finished @rdeago's task in 1m 11s —— View job

---

Follow-up Review

• Gather context and understand request
• Read the updated test file
• Verify new tests cover the suggested cases
• Post review findings

---

The new tests are exactly right. Here's the breakdown:

Test	What it covers
ResolvePushTarget_Throws_WhenPrereleaseFeedHasNoSource	Prerelease feed present but source absent → throws with prerelease-channel name in message; does not fall back to release
ResolvePushTarget_Throws_WhenPrereleaseFeedHasNoApiKeyEnv	Same, apiKeyEnv absent
ResolvePushTarget_Throws_WhenReleaseFeedHasNoSource	Release feed present but source absent → throws
ResolvePushTarget_Throws_WhenReleaseFeedHasNoApiKeyEnv	Same, apiKeyEnv absent
ResolvePushTarget_Throws_WhenApiKeyEnvVarIsUnset	Feed config is complete but the named env var doesn't exist at resolution time → throws

The tests for the prerelease-no-fallback case are particularly good: both include a valid release feed alongside the broken prerelease one, which explicitly confirms the intent — a present-but-broken prerelease feed does not silently fall back to release, it fails fast with a channel-named error.

The full matrix is now covered cleanly across all three routing branches (prerelease, prerelease-fallback, release) and all three failure modes (no feed, no source, no apiKeyEnv, env var absent).

LGTM — nothing left to add.