Fallout-build / Fallout-build/Fallout

Refactor Host activation: replace by-name reflection with a typed HostRegistry

Open
#426 0 comments 0 reactions 0 assignees View on GitHub
enhancement target/vCurrent
Dominant language
C#
Stars
154
Forks
19
Avg merge
1d 22h
Merged PRs (30d)
15

Description

### Problem
`Host` selection picks the active host (Terminal/IDE/CI) through the same reflection family we just removed from the CLI in #392, one layer down in the build engine:

- **By-name property reflection** — `Host.Default` calls `IsRunning(Type)`, which does `hostType.GetProperty($"IsRunning{hostType.Name}")` and `.NotNull()`s the result (`src/Fallout.Build/Host.Activation.cs:31-33`). A renamed/missing `IsRunning{Name}` predicate is a **runtime** failure, not a compile error — the exact stringly-typed brittleness #392 eliminated.
- **Assembly scan** — `AvailableTypes` scans `AppDomain.CurrentDomain.GetAssemblies().SelectMany(GetTypes)` for `Host` subclasses (`Host.Activation.cs:23-27`).
- **Untyped construction** — `Activator.CreateInstance(hostType, nonPublic: true)` (`Host.Activation.cs:39`).

The convention is also **duplicated in tests** (`tests/Fallout.Common.Tests/CITest.cs:117-120` re-implements the same `IsRunning{Name}` reflection) — the tell-tale sign of a missing first-class contract.

> [!NOTE]
> **Hard constraint discovered while scoping:** `Host`/`Terminal` compile into **`Fallout.Build`**, but the 11 CI hosts compile into **`Fallout.Common`**, which references `Fallout.Build` (`Fallout.Common.csproj:13`). The dependency runs `Fallout.Common → Fallout.Build`, so `Host` *cannot* reference its CI subclasses at compile time. **This is why the assembly scan exists**, and it rules out a single central registry listing every host (would be a circular reference). The scan stays; only the name-convention reflection and untyped construction go.

### Outcome
A typed, push-based registry replaces by-name reflection — the engine-side analog of #392's `IFalloutCommand`/dispatcher. No DI **container** (the build engine has none today, and host selection has no dependency graph — a container would be disproportionate); the registry *is* the "typed contract + registration" essence of #392, and `HostRegistry.Register(...)` doubles as a plugin-registration primitive for milestone #6.

```csharp
internal sealed record HostRegistration(
string Name,
int Priority, // higher wins; base Terminal = 0 (guaranteed fallback)
Func IsRunning, // detection BEFORE construction — no env side effects
Func Create); // typed, compile-checked construction

internal static class HostRegistry
{
internal static void Register(HostRegistration registration);
internal static IReadOnlyList Registrations { get; }
internal static IEnumerable Names => Registrations.Select(x => x.Name);
}
```

Each assembly registers its own hosts via a `[ModuleInitializer]` (Terminal/IDE in `Fallout.Build`; CI hosts in `Fallout.Common`), reusing the existing `IsRunning*` predicates as the `Func` (referenced, not reflected). `Host.Default` collapses to pure LINQ over the registry, preserving today's ordering (CI > IDE-terminal > base `Terminal`):

```csharp
internal static Host Default =>
HostRegistry.Registrations
.OrderByDescending(x => x.Priority)
.First(x => x.IsRunning()) // base Terminal.IsRunning == true → always resolves
.Create();
```

The `[TypeConverter]` for `--host ` (`Host.Activation.cs:42-65`) resolves by registered `Name` instead of `Type.FullName.EndsWith`, preserving the string-matching CLI surface.

This is **non-breaking** → targets `main` / `target/2026` (unlike #392, no `experimental`/`breaking-change`). The entire public surface is preserved: `Host`, all subclasses, `public new static T Instance`, the public `Terminal.IsRunningTerminal`, the `Host` parameter, and `--host ` behavior. Everything changed is `internal` (`AvailableTypes`, `Default`, the reflection helpers).

### Acceptance criteria
- [ ] `HostRegistration` + `HostRegistry` replace `AvailableTypes` reflection, `IsRunning(Type)` by-name lookup, and `Activator.CreateInstance` in `Host.Default`/`TypeConverter`
- [ ] Every built-in host (4 in `Fallout.Build`, 11 CI in `Fallout.Common`) is registered via per-assembly `[ModuleInitializer]`; no `IsRunning{Name}` reflection remains in the activation path
- [ ] Host-selection ordering, `--host ` spellings, and the public surface (`Host`, subclasses, `*.Instance`, `Terminal.IsRunningTerminal`, `Host` parameter) are unchanged
- [ ] `FalloutBuild.HostNames` + `SchemaUtility` read `HostRegistry.Names`; `CITest.cs` detection migrates off its duplicated reflection onto the registry
- [ ] Registry selection has unit-test coverage (`HostRegistryTests`: priority ordering, single-running detection, `Terminal` fallback, `--host` resolution, unknown-name, dedup)

### Approach
Incremental, mirroring the #392 stack — each PR builds green and diffs only against its parent:

1. **Foundation** — add `HostRegistration`/`HostRegistry`; register the 4 `Fallout.Build` hosts; rewrite `Host.Default` + `TypeConverter` to consult the registry **with a transitional fallback to the old reflection** for not-yet-registered types (the `DelegateCommand` analog). Add `HostRegistryTests`.
2. **Register CI hosts** — `Fallout.Common` `[ModuleInitializer]` registers all 11; every host now registry-resolved.
3. **Delete the reflection** — remove `AvailableTypes`, `IsRunning(Type)`, `CreateHost`, and the transitional fallback; repoint `HostNames`/`SchemaUtility` to `HostRegistry.Names`.
4. **Collapse tests** — migrate `CITest.cs:117-120` onto `HostRegistry`, killing the duplicated convention.

**Open decision (needs a call before coding):** push-based registry (above, recommended — zero reflection, least host churn, plugin-registration primitive) **vs.** scan-discovered `internal interface IHostDetector { bool IsRunning; int Priority; Host Create(); }` (keeps the assembly scan, drop-in detectors, but ~15 new types and still `Activator`-constructs the detectors). Module initializers are new to this repo; the `IHostDetector` variant avoids them if that's preferred.

Sibling of #392 (CLI dispatch); same "reflection → typed contract" refactor applied to the engine's host activation.

Contributor guide

Open the contributing guide

Research direction

Start with src/Fallout.Build/Host.Activation.cs and review Fallout.Common.csproj to understand the assembly boundary, then inspect the four Fallout.Build hosts and the 11 CI hosts in Fallout.Common. Resolve the open registry approach before coding. Done means typed registration replaces activation reflection, ordering and --host names remain unchanged, HostRegistryTests cover selection, and CITest.cs, FalloutBuild.HostNames, and SchemaUtility use the registry.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
build-system
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.