Consolidate the build pipeline into a cohesive BuildPipeline

[rdeago]

Summary

Prerequisite for #269. Extracts the build pipeline (Clean → Restore → Build → Test → Pack)
into a real domain object so #269 can later hang buildvana.json settings onto pipeline steps.

• Add an ordered BuildStep enum and an injectable BuildPipeline in a new
  Buildvana.Tool.Build namespace. BuildPipeline owns the step bodies (moved from the static
  BuildSteps) and exposes the three execution shapes: a single step (RunAsync), a prefix
  (RunThroughAsync), and a range (RunRangeAsync).
• Rewire the five pipeline subcommands to drive BuildPipeline via RunThroughAsync(BuildStep.X).
• Rewire ReleaseCommand to drive the pipeline step-by-step, preserving its two passes
  (Clean→Test verification, then Restore→Pack artifacts with no re-clean).
• Register BuildPipeline in DI; delete BuildSteps.

Strictly behavior-preserving: same dotnet invocations, in the same order, with the same
arguments and side effects. No buildvana.json wiring (that's #269), no new subcommand,
no public API change.

Test plan

• dotnet bv build — clean, zero warnings
• dotnet bv pack — full Clean→Restore→Build→Test→Pack ran in order, tests pass, both packages produced
• ReSharper inspectcode --severity=WARNING — 0 results

No CHANGELOG entry: internal refactor, no observable behavior change.

Closes #280.

[rdeago]

@claude please review this PR.

[claude]

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

---

Code Review

• Gather context
• Read changed files
• Analyze changes
• Post review

---

The refactor is clean and behavior-preserving. I verified all five pipeline commands and ReleaseCommand against the deleted BuildSteps.cs — step sequences are identical in every code path.

Behavioral equivalence check

Command	Old sequence	New sequence
clean	CleanAsync	RunThroughAsync(Clean) = Clean
restore	Clean, Restore	RunThroughAsync(Restore) = Clean→Restore
build	Clean, Restore, Build	RunThroughAsync(Build) = Clean→Restore→Build
test	Clean, Restore, Build, Test	RunThroughAsync(Test) = Clean→Restore→Build→Test
pack	Clean, Restore, Build, Test, Pack	RunThroughAsync(Pack) = Clean→Restore→Build→Test→Pack
release pass 1	Clean, Restore, Build(cfg), Test(cfg)	RunThroughAsync(Test, cfg)
release pass 2	Restore, Build(cfg), Test(cfg), Pack(cfg)	RunRangeAsync(Restore, Pack, cfg)

All match. The two-pass nature of ReleaseCommand is preserved correctly.

Design

The refactor is a genuine improvement: BuildPipeline is a proper DI-friendly object, constructor guards are in place, DefaultConfiguration moved with the logic that owns it, and callers are dramatically simplified. The BuildStep enum as the iteration key is elegant — the step++ idiom over an ordered enum is idiomatic and safe.

Findings

Minor — RunRangeAsync silently no-ops if first > last (BuildPipeline.cs:70-76)

for (var step = first; step <= last; step++)

If a caller passes first > last, the loop exits immediately with no steps run and no error. There are no such callers today, but given that RunRangeAsync is public and the enum values are plain integers, this is a quiet footgun. A guard like Guard.IsLessThanOrEqualTo((int)first, (int)last, nameof(first)) (or a swap-and-warn strategy) would make misuse obvious. Not a bug in current usage, just something to decide on before #269 adds more callers.

Pre-existing — cancellationToken is received but never forwarded (all command files)

Every ExecuteAsync(CancellationToken cancellationToken) discards its token — it's never passed to pipeline.RunThroughAsync/RunRangeAsync or any DotNetService method. This predates the PR and isn't worth blocking on here, but it's worth a follow-up issue if dotnet child processes should be cancellable.

Pre-existing — TestCommand description is slightly inaccurate (TestCommand.cs:13)

[Description("Build all projects and run tests.")]

The command actually does Clean → Restore → Build → Test, so "Build" alone undersells it. Again predates the PR; just noting it since BuildCommand and RestoreCommand descriptions are already accurate.

Summary

Code is correct, clean, and well-structured. The only item worth acting on before #269 is the RunRangeAsync(first > last) guard decision — everything else is pre-existing or cosmetic. Happy to approve once you've considered that point.

[rdeago]

Thanks for the review! Addressed the one actionable point:

RunRangeAsync inverted-range footgun — fixed in 18bf1e8. RunRangeAsync now guards with Guard.IsLessThanOrEqualTo((int)first, (int)last, nameof(first)), so first > last throws ArgumentOutOfRangeException instead of silently running nothing. Consistent with the constructor's existing Guard.* use; build is clean.

The two pre-existing findings I'm intentionally leaving out to keep this PR a behavior-preserving refactor:

• cancellationToken never forwarded — agreed, worth a follow-up issue. Threading it through BuildPipeline and DotNetService is a genuine behavior change and belongs in its own PR.
• TestCommand description — also pre-existing. It's user-facing help text, so changing it here would conflict with this PR's "no observable change / no CHANGELOG entry" framing. Better as a small standalone fix.

@claude please re-review.

[claude]

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

---

Re-Review

• Gather context
• Read updated files
• Verify the RunRangeAsync guard fix
• Post final review

---

The fix in 18bf1e8 is exactly right.

BuildPipeline.cs:72:

Guard.IsLessThanOrEqualTo((int)first, (int)last, nameof(first));

This is consistent with the constructor's existing Guard.* usage, throws ArgumentOutOfRangeException immediately on misuse, and makes the contract explicit. No issues.

The two deferred pre-existing findings are reasonable calls — both belong in their own PRs and don't affect this refactor's correctness. LGTM.