Move ParameterService onto the BuildContext (FT-4)#452
Open
ChrisonSimtian wants to merge 2 commits into
Open
Conversation
This was referenced Jun 30, 2026
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.
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.
ChrisonSimtian
force-pushed
the
engine/ft4-parameter-service
branch
from
July 26, 2026 10:17
119fee5 to
69b4088
Compare
ChrisonSimtian
marked this pull request as ready for review
July 26, 2026 10:18
ChrisonSimtian
added a commit
to ChrisonSimtian/Fallout
that referenced
this pull request
Jul 26, 2026
Runs a fixture build end-to-end, twice in one process, and asserts what the de-static work promised: both runs succeed, the second gets its own context/parameter service/log sink, it opens on an empty sink, and the static facades inside a run point at that run. This is what the original FT-9 commit set out to do. Its unit-level assertions have since landed with the steps they belong to (FT-2 in Fallout-build#451, FT-4 in Fallout-build#452, FT-6 in Fallout-build#453), so what is left here is the end-to-end case they could not cover — and it immediately found the leaked file-sink logger fixed in the previous commit. Verified as a real guard: reverting that fix fails 4 of these 5 specs. Two process-wide inputs are pinned to make a build runnable under a test host — the argument parser (swapped and restored) and the statically-resolved root directory (asserted to carry the .fallout marker, since UpdateNotification prompts for input without it). Both are de-statification candidates in their own right.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves
ParameterServicefrom a static singleton ontoBuildContext, so each build run gets its own instance instead of sharing process-wide state.What changed
Parametersproperty toBuildContextholding a per-runParameterService.ParameterService.Instancebecomes a facade overBuildContext.Current.Parameters, with a lazy ambient fallback for use outside a run.BuildContextSpecs: the facade resolves the running context's instance, falls back to a stable instance outside a run, each run gets its own service, and a mutated per-run field doesn't leak into the next run.Why
Production and the specs now use the same instance form — the specs already built
ParameterServicedirectly, but production relied on the static singleton. The singleton's mutable fields (ArgumentsFromFilesService,ArgumentsFromCommitMessageService) used to live for the whole process, so state could leak between runs. Scoping the service toBuildContextcloses that gap.The ambient ("ready-to-use without setup") fallback stays process-wide for now, since there's no cross-run state left to leak on that path. It can be removed once we confirm nothing uses
ParameterServiceoutside a run.Non-breaking — the static
ParameterServiceAPI is unchanged.Part of #309 (FT-4, the third step of the engine de-statification work), following #450 (FT-1) and #451 (FT-2, which set up the pattern this PR reuses). Part of the epic #315.
Verification
Full build + full spec suite green (14 projects, 0 failures).