From f0f394c2ec7809a83b314f873e39a4c9c23ecd5b Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Tue, 30 Jun 2026 22:06:35 +1200 Subject: [PATCH 1/2] Move ParameterService onto the BuildContext (FT-4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The active ParameterService is now the per-run instance held on BuildContext.Parameters; the static ParameterService.Instance becomes a facade over BuildContext.Current. This is the first service to ride the FT-2 rail: production and tests now exercise the same instance form (killing the test/prod divergence — tests already `new ParameterService(funcs)`), and the per-run instance + its mutable fields (ArgumentsFromFilesService / ArgumentsFromCommitMessageService) no longer leak across runs. A lazy fallback covers access outside a build run (no cross-run state to leak there); it can retire once that path is confirmed dead. Non-breaking — the static API is unchanged. --- src/Fallout.Build/Execution/BuildContext.cs | 12 +++++++++++- .../Execution/ParameterService.Statics.cs | 12 +++++++++--- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/Fallout.Build/Execution/BuildContext.cs b/src/Fallout.Build/Execution/BuildContext.cs index aeb4256a6..57cefa400 100644 --- a/src/Fallout.Build/Execution/BuildContext.cs +++ b/src/Fallout.Build/Execution/BuildContext.cs @@ -21,7 +21,8 @@ namespace Fallout.Common.Execution; /// /// FT-2 / #307. Intentionally /// internal — not a public contract until the SDK lands (milestone #7). Subsequent steps move -/// the per-run services (parameters, logging scope, tool-path config) onto this context. +/// the remaining per-run services (logging scope, tool-path config) onto this context; the parameter +/// service already rides it (FT-4 / #309). /// internal sealed class BuildContext : IDisposable { @@ -34,6 +35,15 @@ internal sealed class BuildContext : IDisposable private readonly ConsoleCancelEventHandler onCancelKeyPress; private readonly EventHandler onToolOptionsCreated; + /// + /// The parameter service for this run. FT-4 / #309: + /// this instance is what the static facade resolves to, so a + /// build reads parameters through the same instance form the specs already use, and the service's + /// mutable fields die with the run instead of leaking into the next one. + /// + public ParameterService Parameters { get; } = + new(() => EnvironmentInfo.ArgumentParser, () => EnvironmentInfo.Variables); + private BuildContext() { onCancelKeyPress = (_, _) => cancellationHandlers.ForEach(x => x()); diff --git a/src/Fallout.Build/Execution/ParameterService.Statics.cs b/src/Fallout.Build/Execution/ParameterService.Statics.cs index e8e026b13..cbb025a9d 100644 --- a/src/Fallout.Build/Execution/ParameterService.Statics.cs +++ b/src/Fallout.Build/Execution/ParameterService.Statics.cs @@ -2,15 +2,21 @@ using System.Linq; using System.Linq.Expressions; using System.Reflection; +using Fallout.Common.Execution; using Fallout.Common.Utilities; namespace Fallout.Common; internal partial class ParameterService { - internal static ParameterService Instance = new( - () => EnvironmentInfo.ArgumentParser, - () => EnvironmentInfo.Variables); + // FT-4 (#309): the active service is the per-run one on BuildContext, so a build reads parameters + // through the same instance form the specs use and nothing leaks into the next run. The fallback + // covers access outside a run — process-wide, but there is no cross-run state to leak there — and + // can retire once that path is confirmed dead. + private static readonly Lazy ambientInstance = new( + () => new ParameterService(() => EnvironmentInfo.ArgumentParser, () => EnvironmentInfo.Variables)); + + internal static ParameterService Instance => BuildContext.Current?.Parameters ?? ambientInstance.Value; public static T GetParameter(string name, char? separator = null) { From 69b40881d30c7c8d30186de4c27c3c568f49e1e0 Mon Sep 17 00:00:00 2001 From: Chrison Simtian Date: Sun, 26 Jul 2026 22:17:38 +1200 Subject: [PATCH 2/2] Cover the per-run parameter service with specs Four cases on the FT-4 seam: the static facade resolves the running context's instance, it falls back to a stable ambient instance outside a run, each run gets its own service, and a mutated per-run field does not survive into the next run. --- .../Fallout.Build.Specs/BuildContextSpecs.cs | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/Fallout.Build.Specs/BuildContextSpecs.cs b/tests/Fallout.Build.Specs/BuildContextSpecs.cs index 73f0c47ba..8f62d0fa5 100644 --- a/tests/Fallout.Build.Specs/BuildContextSpecs.cs +++ b/tests/Fallout.Build.Specs/BuildContextSpecs.cs @@ -44,6 +44,52 @@ public void Dispose_clears_current() BuildContext.Current.Should().BeNull(); } + [Fact] + public void Parameter_service_facade_resolves_the_current_contexts_instance() + { + using var context = BuildContext.Activate(); + + // FT-4 (#309): the static facade is a pointer at the running context, not its own singleton. + ParameterService.Instance.Should().BeSameAs(context.Parameters); + } + + [Fact] + public void Parameter_service_facade_falls_back_to_an_ambient_instance_outside_a_run() + { + BuildContext.Current.Should().BeNull(); + + // Nothing to route to outside a run, so the facade hands back a stable process-wide instance + // rather than throwing — this is the path the fallback exists for. + ParameterService.Instance.Should().NotBeNull(); + ParameterService.Instance.Should().BeSameAs(ParameterService.Instance); + } + + [Fact] + public void Each_run_gets_its_own_parameter_service() + { + ParameterService first; + ParameterService second; + + using (var context = BuildContext.Activate()) + first = context.Parameters; + using (var context = BuildContext.Activate()) + second = context.Parameters; + + second.Should().NotBeSameAs(first); + } + + [Fact] + public void Parameter_service_state_does_not_leak_into_the_next_run() + { + using (BuildContext.Activate()) + ParameterService.Instance.ArgumentsFromCommitMessageService = new ArgumentParser(["-arg", "value"]); + + // The mutable per-run fields (commit-message args, args-from-files) died with the previous + // context — the whole point of moving the service off a process-global singleton. + using (BuildContext.Activate()) + ParameterService.Instance.ArgumentsFromCommitMessageService.Should().BeNull(); + } + [Fact] public void Disposing_a_superseded_context_leaves_the_newer_one_current() {