Add IReporter-over-MSBuild adapter and BuildvanaSdkTask.Reporter

[rdeago]

Closes #294.

Implements the plumbing for running Core services inside MSBuild tasks (design option (b) of #267): human-facing output flows through IReporter, bridged to the build's own loggers.

Changes

• TaskLoggingHelperReporter (internal, Buildvana.Sdk.Tasks) — an IReporter backed by the task's TaskLoggingHelper, sibling to the existing TaskLoggingHelperLogger ILogger bridge:
  • Report: Error → LogError, Warning → LogWarning, Info/Detail/Trace → LogMessage at High/Normal/Low importance.
  • Activity scopes render the same [depth] title: starting.../done (X.Xs) lines as ConsoleReporter (outcome only after Complete), at Normal importance so they stay quiet at MSBuild's default (minimal) verbosity.
  • ChildOutput/ChildError both pass through as low-importance messages: MSBuild has no neutral stderr channel, and a build error would fail the task over stderr lines many tools use for progress.
  • Verbosity is derived from IBuildEngine10.EngineServices importance filtering when available (fully permissive fallback), so ReporterExtensions formatting helpers skip messages MSBuild would discard; MSBuild's own verbosity gating governs visibility either way.
• BuildvanaSdkTask exposes a lazily built protected IReporter Reporter, mirroring Logger, for derived tasks to hand to Core services. No task consumes it yet; existing tasks are unchanged.
• Tests — new Buildvana.Sdk.Tasks.Tests project (TUnit): a recording IBuildEngine (plus an IBuildEngine10 variant with stubbed EngineServices) drives the full mapping through the real Reporter property of a probe task; 20 tests cover levels, importances, activity lifecycle, child passthrough, and verbosity derivation.

No CHANGELOG entry: internal-only, no public-facing change.

Validation

• dotnet build: zero warnings/errors; all tests pass.
• dotnet bv pack self-hosts and produces the Buildvana.Sdk and bv packages.
• ReSharper inspectcode (--swea --severity=WARNING): no findings in changed code (one pre-existing false positive in Buildvana.Core.JsonSchema, untouched by this PR).

🤖 Generated with Claude Code

[rdeago]

@claude please review this PR.

[claude]

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

---

Review

• Gather context (PR description, changed files)
• Read implementation files (TaskLoggingHelperReporter*.cs, BuildvanaSdkTask.cs)
• Read test files
• Check adherence to repo style guide / architecture rules
• Post review feedback

Overall this is clean, well-scoped work: it mirrors the existing TaskLoggingHelperLogger bridge closely, stays entirely internal (no public API surface, no CHANGELOG entry needed), and is backed by 20 tests that actually exercise the real TaskLoggingHelper/IBuildEngine10 plumbing rather than mocking it away. CI (Build, test, and pack + CodeQL) is green.

A few notes, nothing blocking:

1. ChildOutput/ChildError are byte-for-byte identical (src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs:98-118). Both just gate on minimumVerbosity and forward to _log.LogMessage(MessageImportance.Low, "{0}", line). Worth collapsing into a shared private helper, e.g.:

private void LogChildLine(string line, Verbosity? minimumVerbosity)
{
    ArgumentNullException.ThrowIfNull(line);
    if (minimumVerbosity is { } v && !this.IsVerbosityAtLeast(v))
    {
        return;
    }

    _log.LogMessage(MessageImportance.Low, "{0}", line);
}

with ChildOutput/ChildError both delegating to it. ConsoleReporter keeps them separate because they write to different streams (Console.Out vs Console.Error), but here they do the exact same thing, so the duplication buys nothing.

2. Slight tension with IReporter.ChildOutput/ChildError's doc contract. The interface says "If null, the line is always written" (src/Buildvana.Core.Abstractions/ConsoleOutput/IReporter.cs:39-40,48-49). In this implementation, even with minimumVerbosity: null, the line can still be silently dropped by TaskLoggingHelper.LogMessage's own importance filtering (confirmed by ChildLines_WhenEngineDiscardsLowImportance_AreNotForwarded). This is clearly deliberate and well explained in the class remarks ("visibility is governed by MSBuild's own verbosity and importance filtering, exactly like any other message logged by a task"), so I don't think it needs to change — just flagging that "always written" isn't quite literally true for this implementation, in case that's worth a one-line caveat in the interface doc for future implementers.

3. Verbosity never reports Verbosity.Quiet (TaskLoggingHelperReporter.cs:52-55) — the lowest it can derive is Minimal. That's correct given MSBuild always surfaces errors/warnings regardless of verbosity (there's no MSBuild equivalent of suppressing warnings), just noting it as intentional asymmetry with ConsoleReporter, not a bug.

4. EndActivity's out-of-order-dispose handling (TaskLoggingHelperReporter.cs:136-154): if an outer activity is disposed while an inner one is still open, the outer scope is neither popped nor logged (only _activityStack.Peek() == scope pops), leaving the stack permanently out of sync for the rest of the task's lifetime. This exactly mirrors ConsoleReporter.EndActivity, so it's pre-existing behavior rather than something new — not asking for a fix here, just noting it's inherited, not introduced.

5. Minor test-coverage gap: the guard clauses (ArgumentNullException/ArgumentException on the constructor, Report's message, BeginActivity's title, ChildOutput/ChildError's line) aren't covered by the new tests. Given how thorough the rest of the suite is (levels, importances, activity lifecycle, verbosity derivation), this stood out as the one gap — low priority, but easy to close if you want full coverage.

No correctness issues found in the level/importance mapping, the activity line formatting (matches ConsoleReporter exactly, byte-for-byte), the lazy Reporter property caching on BuildvanaSdkTask, or the file-format/style-guide compliance (BOM present on all new .cs files, partial-class split for the nested ActivityScope, package versions already centrally managed — no Directory.Packages.props changes needed).
· branch

[rdeago]

Thanks for the review! Addressed in 8d3787a:

1. ChildOutput/ChildError duplication — collapsed into a shared LogChildLine private helper; both methods now delegate to it.
2. IReporter doc contract — reworded the minimumVerbosity docs on both methods: with null the reporter applies no verbosity gate of its own, but an underlying output system may still apply its own filtering.
3. Verbosity never reporting Quiet — intentional, as you noted; no change.
4. EndActivity out-of-order-dispose handling — inherited from ConsoleReporter, as you noted; no change.
5. Guard-clause coverage — added tests for the constructor and all argument guards (7 new tests, 27 total). Testing the internal constructor required an InternalsVisibleTo("Buildvana.Sdk.Tasks.Tests") on Buildvana.Sdk.Tasks, mirroring the existing Buildvana.Tool pattern.

@claude please take another look.

[claude]

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

---

Follow-up review

• Read trigger comment and prior review
• Review commit 8d3787a (fixes for items 1, 2, 5)
• Check new tests for guard clauses
• Verify build/tests (CI green on the new commit)
• Post final feedback

All three addressed points check out:

1. LogChildLine collapse (src/Buildvana.Sdk.Tasks/TaskLoggingHelperReporter.cs:98-100,118-127) — ChildOutput/ChildError are now one-line delegations to a shared private helper. Clean, no behavior change.
2. IReporter doc wording (src/Buildvana.Core.Abstractions/ConsoleOutput/IReporter.cs:39-41,49-51) — the reworded contract ("the reporter applies no verbosity gate of its own; an underlying output system may still apply its own filtering") is accurate and matches what TaskLoggingHelperReporter actually does. Good fix.
3. Guard-clause coverage — the 7 new tests (TaskLoggingHelperReporterTests.cs:10-22,70-75,143-169) cover the constructor's log/engine null checks, Report's null message, BeginActivity's null/empty title, and ChildOutput/ChildError's null line. That closes the gap I flagged. Using ReporterProbeTask/RecordingBuildEngine for the constructor tests (rather than mocking TaskLoggingHelper directly) is consistent with the rest of the suite's "exercise the real thing" approach. The InternalsVisibleTo addition to make the internal constructor testable mirrors the existing Buildvana.Tool pattern, as noted.

Items 3 (Verbosity never reporting Quiet) and 4 (EndActivity out-of-order-dispose) were correctly left as-is — both are intentional/inherited behavior, not bugs.

CI (Build, test, and pack + CodeQL) is green on 8d3787a. No new issues found — this looks ready to merge.
· branch