Fallout-build / Fallout-build/Fallout
Put an ILogger abstraction over Serilog and unify the two console output paths
- Dominant language
- C#
- Stars
- 154
- Forks
- 19
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 15
Description
## Description
Refactor Fallout's console/logging output to sit behind a clean abstraction and collapse the **two parallel output paths** that exist today.
### Problem
Serilog is already the core logger — `Log.*` runs a real Serilog pipeline (console / host-CI / in-memory / file sinks, configured in `src/Fallout.Build/Logging.cs::Configure`). But **all UI/presentation output bypasses Serilog**: the logo, the target-outcome table, `WriteBlock` banners, build success/fail lines, and the "Errors & Warnings" block call static `Host.Success/Information/...` (`src/Fallout.Build/Host.Theming.cs`) → `IHostTheme.Write*` → raw `Console.WriteLine(Format(text, ansiCode))` (`src/Fallout.Build/Theming/AnsiConsoleHostTheme.cs:106`).
The seam joining the two paths is `IHostTheme`, which does awkward double duty: it implements the direct-write methods **and** inherits Serilog's `ConsoleTheme` so it can be cast `(ConsoleTheme)Theme` and handed to the console sink (`Logging.cs:103`).
Resulting smells:
- `Host.WriteErrorsAndWarnings` (`Host.cs:85`) **rebuilds the global `Log.Logger`** just to flush in-memory events — there is a literal `// TODO: move to Logging`.
- Direct-write UI lines do **not** reach the file/CI sinks and are **not** subject to the level switch / verbosity, unlike `Log.*` output — inconsistent behaviour for "the same" output.
- Serilog types **leak into the public API**: `Logging.LevelSwitch` (public `LoggingLevelSwitch`), the `ConsoleTheme` base classes on the public theme types, and `LogEventLevel` in the public `LogLevelPattern` / `DefaultLogLevel` attributes (`src/Fallout.Tooling/ToolTasks.Logger.cs`).
### Colour requirement (resolved)
Colourful console output is a hard requirement. Findings:
- **`ILogger` loses no colour** — colour is a provider/formatter concern, not an `ILogger`-API concern.
- **Serilog's console sink already does rich per-token colour** (ANSI/256/truecolor) but **not layout** (tables/boxes/progress) — which is exactly why the rich UI bypasses it today.
- **A custom `ILoggerProvider` was rejected** — zero colour benefit, and it would reimplement everything Serilog already gives us (level switch, in-memory capture, CI-annotation sink, file sinks + rolling cleanup, enrichers, filters).
- **Spectre.Console** (already a pinned dependency, used by the CLI prompts) is the right tool for the rich-UI part.
### Target architecture (hybrid)
1. **Logging** — code against `Microsoft.Extensions.Logging.ILogger`, keeping **Serilog as the provider** behind it via `Serilog.Extensions.Logging` (`SerilogLoggerFactory`). The existing Serilog pipeline is unchanged.
2. **Theming** — keep an `IHostTheme`-style interface but **decouple it from Serilog** (stop inheriting `ConsoleTheme`); add an internal `SerilogConsoleThemeAdapter : ConsoleTheme` that wraps the theme's colour data for the sink.
3. **Rich UI** — move the logo / outcome table / banners to **Spectre.Console** behind a new internal `IHostOutput` presenter, eliminating the direct-`Console.WriteLine` bypass and fixing `WriteErrorsAndWarnings`.
### Key decisions
- **Façade-over-DI, not full constructor injection.** The `IFalloutCommand` + MS.DI container introduced in #d0452cae lives **only in `Fallout.Cli`** — the build engine (`BuildManager.Execute`) is fully static (`new T()`, static `Log` / `Host.Instance`, ~50 call sites). So: introduce a composition root inside `BuildManager.Execute` (a per-run `ServiceCollection` + a shared `AddFalloutLogging` extension, also registered in the CLI container), but **expose the resolved logger / theme / output through the existing static façades** so the ~50 call sites are unchanged.
- **Adopt Spectre.Console now** (not deferred) for the rich-UI rendering.
### Additive-now vs deferred-breaking
Per the backwards-compatibility rules in `AGENTS.md` (#1/#2), removing the public Serilog types is a **breaking change** and must batch to the next yearly major on `experimental`.
**Additive now (`main`, `target/2026`):**
- ILogger seam + composition root + `AddFalloutLogging`; log façade re-pointed (Serilog `Log.*` keeps working).
- Decoupled `IHostTheme` colour model + `SerilogConsoleThemeAdapter` (keep the old `ConsoleTheme` base classes in place so the cast still compiles).
- Spectre `IHostOutput` presenter; fix `WriteErrorsAndWarnings` (no `Log.Logger` rebuild).
- `[Obsolete]` / `[Experimental("FALLOUT002")]` markers on the Serilog leaks; add `Fallout.Common.LogLevel` overloads to `LogLevelPattern` / `DefaultLogLevel`; register the experimental ID in `docs/experimental-apis.md`.
**Deferred to `experimental` (breaking, next yearly major):**
- Remove `ConsoleTheme` base classes from the `IHostTheme` impls; remove `public LevelSwitch`; remove `LogEventLevel` / `ConsoleThemeStyle` from public signatures. Each gets the `breaking-change` label + `⚠️ Breaking change` callout + CHANGELOG entry under the next major.
### Proposed PR sequence
1. **(main, additive)** Packages (`Serilog.Extensions.Logging`, `Microsoft.Extensions.Logging.Abstractions`); `AddFalloutLogging` + `ILoggerFactory` registration backed by the existing Serilog pipeline; composition root in `BuildManager.Execute`; static log façade re-pointed. No behaviour change. + bridge tests.
2. **(main, additive)** Decoupled `IHostTheme` colour model + `SerilogConsoleThemeAdapter`; rewire `Logging.cs:103` / `Host.cs:96` to use the adapter. Adapter parity tests + baseline Verify snapshots.
3. **(main, additive)** `IHostOutput` Spectre presenter for logo/table/banners/outcome; fix `WriteErrorsAndWarnings`; Spectre `PackageReference` on `Fallout.Build`; CI host overrides updated. Verify-snapshot review.
4. **(main, additive)** `[Obsolete]` / `[Experimental("FALLOUT002")]` markers + new `LogLevel` attribute overloads; register `FALLOUT002` in `docs/experimental-apis.md`; wire the CLI container to the same registrations.
5. **(experimental, breaking — next major)** Remove `ConsoleTheme` base classes, `public LevelSwitch`, and all `LogEventLevel` / `ConsoleThemeStyle` from public signatures.
### Packages (central — `Directory.Packages.props`)
- **New:** `Serilog.Extensions.Logging`, `Microsoft.Extensions.Logging.Abstractions`.
- **Already present (add refs to `Fallout.Build`):** `Microsoft.Extensions.DependencyInjection`, `Spectre.Console`.
### Test strategy
- **Verify snapshots are the regression contract** for every output-format change (logo / table / banner) in ANSI, plain, and redirected modes.
- `SerilogConsoleThemeAdapter` parity tests (same ANSI codes per level/token as today).
- `ILogger` → Serilog bridge tests (correct `LogEventLevel` mapping).
- `LogLevelPattern` / `DefaultLogLevel` overload parity tests.
- `WriteErrorsAndWarnings` test asserting no `Log.Logger` reset.
### Risks / open questions
- The composition root in `BuildManager.Execute` is **new architecture**, not just rewiring.
- Spectre truecolor **downgrade** for the Fallout-yellow logo (`#F5C800`, `Host.cs:36`) must match the current "older sinks ignore the escape bytes" intent.
- `applyThemeToRedirectedOutput: true` (`Logging.cs:104`, `Host.cs:97`) must be preserved — CI logs rely on it.
- 7 CI `*.Theming.cs` overrides (esp. GitHub Actions `::`-prefixed annotations) must not be stranded by the `IHostOutput` split.
- `InMemorySink` is a process-wide singleton (`Logging.cs:214`) — confirm lifetime under per-run providers so the end-of-build summary still captures everything.
- Confirm `FALLOUT002` is the next free experimental diagnostic ID before allocating.
## Usage Example
No change to the public logging surface in the additive phases — existing `Log.Information(...)` and the colored build UI keep working. Internally, components resolve logging through the abstraction instead of touching Serilog directly:
```csharp
// Composition root (BuildManager.Execute) — wired once, façades fed from it
services.AddFalloutLogging(build); // registers ILoggerFactory (Serilog provider), IHostTheme, IHostOutput
// Theme decoupled from Serilog; sink still gets colour via an adapter
configuration.WriteTo.Console(theme: new SerilogConsoleThemeAdapter(hostTheme), ...);
// Rich UI renders through the presenter instead of raw Console.WriteLine
hostOutput.WriteTargetOutcome(build); // Spectre table, not hand-rolled PadRight + ANSI
```
## Alternative
- **Status quo (the hybrid bypass):** keep two output paths — inconsistent sink coverage, the `Log.Logger`-rebuild hack, and the dual-hat `IHostTheme`/`ConsoleTheme`.
- **Push everything through a Serilog sink:** fights the log pipeline for non-log UI (tables/banners) and deepens the public Serilog coupling that we want to be able to unwind.
- **Write a fully custom `ILoggerProvider`:** rejected — reimplements Serilog's sink fan-out / enrichment / capture for no colour benefit.
Contributor guide
Research direction
Start with AGENTS.md, then trace BuildManager.Execute, src/Fallout.Build/Logging.cs, Host.cs, and Host.Theming.cs to understand the existing composition and output paths. Work through the proposed phases, beginning with bridge tests and existing Verify snapshots; done means the listed tests and snapshots cover ILogger mapping, theme parity, rich output modes, and no Log.Logger reset.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- build-system, observability-sre, tooling
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100