Add a typed step-injection pipeline to the GitHub Actions generator - #475
Conversation
Introduce the public surface a build uses to inject custom workflow steps: GitHubActionsCustomStep (uses:/run: with fixed-order YAML rendering), GitHubActionsStepPosition (PostCheckout/PreRun/PostRun/JobEnd), the IConfigureGitHubActions hook interface, and GitHubActionsStepPipeline (per-job context + Insert + read-only BuiltInSteps view). Expose Fallout.Common internals to Fallout.Common.Specs so the pipeline's generator-only surface stays internal but testable.
Refactor GitHubActionsAttribute.GetSteps to assemble the base step sequence, hand it to a per-job GitHubActionsStepPipeline as read-only BuiltInSteps, query Build for IConfigureGitHubActions, validate the collected inserts, then splice them in at their anchor positions (call order preserved). Validation fails the build on an invalid step: exactly one of Uses/Run, With requires Uses, Shell only on a run step. A build not implementing the interface, or one that inserts nothing, yields byte-identical YAML.
Read the Write flow top-down; the Scalar/MapBlock local functions that close over the list-marker flag now sit at the end. Behaviour is unchanged.
Use raw string literals for the multi-line expected YAML (more readable than line concatenation) and normalize line endings on both sides. The renderer emits Environment.NewLine, so normalizing keeps these exact-output assertions passing on the cross-platform post-merge jobs, not just the ubuntu PR gate.
|
nice feature, thanks for bringing this in :-) |
ChrisonSimtian
left a comment
There was a problem hiding this comment.
Really clean, additive design — the generator keeps sole control of the base sequence and the hook only inserts. Test coverage is the standout: all four positions, the cache/artifacts-absent anchoring case, ordering, workflow scoping, and a byte-identical no-op regression guard.
Approving — nothing here blocks, but one is worth fixing before this gets real-world use (inline): custom-step scalar values are emitted unquoted, whereas every built-in step quotes names, so a Name/If/Uses containing a colon-space, #, or leading indicator char emits invalid YAML. That contradicts the "never emits invalid YAML" line, which today only covers the structural uses/run checks. Two smaller notes on dictionary ordering and a null-step guard.
Confirmed Assert.True throws ArgumentException (src/Fallout.Utilities/Assert.cs), so the validation specs assert the right type. 👍
There was a problem hiding this comment.
Pull request overview
Adds a public, typed step-injection hook to the GitHub Actions generator so builds can insert custom uses: / run: steps at fixed, named positions per job, while keeping the generator’s base step sequence intact.
Changes:
- Introduces the step-injection API surface (
IConfigureGitHubActions,GitHubActionsStepPipeline,GitHubActionsCustomStep,GitHubActionsStepPosition). - Extends
GitHubActionsAttributegeneration to invoke the hook per job, validate injected steps, and splice inserts into the rendered step list. - Adds specs + snapshot coverage for ordering, anchor positions (with/without cache/artifacts), scoping via pipeline context, and validation failures.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Fallout.Common.Specs/CI/GitHubActionsStepPipelineSpecs.cs | Unit coverage for pipeline context exposure + insertion ordering semantics. |
| tests/Fallout.Common.Specs/CI/GitHubActionsStepInjectionSpecs.cs | Behavioral coverage of injection positions, ordering, scoping, and no-op regression. |
| tests/Fallout.Common.Specs/CI/GitHubActionsStepInjectionSpecs.Rich_injection_renders_expected_yaml.verified.txt | Snapshot contract for rich multi-position injection YAML output. |
| tests/Fallout.Common.Specs/CI/GitHubActionsCustomStepValidationSpecs.cs | Verifies invalid injected steps fail generation with ArgumentException. |
| tests/Fallout.Common.Specs/CI/GitHubActionsCustomStepSpecs.cs | Isolated rendering specs for GitHubActionsCustomStep YAML output. |
| src/Fallout.Common/Fallout.Common.csproj | Adds InternalsVisibleTo for specs to access internal pipeline APIs. |
| src/Fallout.Common/CI/GitHubActions/IConfigureGitHubActions.cs | New build hook interface invoked per generated job. |
| src/Fallout.Common/CI/GitHubActions/GitHubActionsStepPosition.cs | New enum defining the four named insertion anchors. |
| src/Fallout.Common/CI/GitHubActions/GitHubActionsAttribute.cs | Hook invocation + splice/validation integrated into job step generation. |
| src/Fallout.Common/CI/GitHubActions/Configuration/GitHubActionsStepPipeline.cs | Pipeline implementation collecting inserts and exposing built-in steps context. |
| src/Fallout.Common/CI/GitHubActions/Configuration/GitHubActionsCustomStep.cs | New user-constructible step type for uses:/run: with optional fields + YAML writer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Quote custom-step names via SingleQuote(), matching the built-in steps, so a name containing a colon stays valid YAML (the structural validation didn't cover value safety). - Render with:/env: in ordinal key order for deterministic output; Dictionary enumeration order isn't guaranteed. - Guard Insert against null steps with Assert.NotNull, and treat null With/Env/Run as empty, so misuse fails with the clean ArgumentException the design promises rather than a NullReferenceException. - Add specs: colon-in-name quoting, multi-entry ordinal ordering, null-step insert, and null collections.
The shared SingleQuote() helper escapes an embedded quote with a backslash,
which is invalid inside a YAML single-quoted scalar, so a name like "Bob's
step" emitted 'Bob\'s step'. Quote the name locally with YAML doubling ('')
instead; output is identical for names without a quote. Left the shared helper
alone — it is also used for shell/log output across 26 call sites where the
doubling form would be wrong.
Also guard the multi-insert ordering test so a dropped first insert fails
instead of passing vacuously on IndexOf == -1.
Move the YAML-correct single-quote (doubling embedded quotes) out of the custom step and into StringExtensions.SingleQuoteYaml, next to SingleQuote/DoubleQuote, with its own unit test. This is the primitive the built-in CI writers should adopt to replace SingleQuote() in YAML contexts (a separate follow-up).
ITaluone
left a comment
There was a problem hiding this comment.
Nice work!
Just this two questions
Switch GitHubActionsCustomStepSpecs from hand-authored expected strings + a line-ending-normalizing helper to Verify, matching the generator testing strategy used elsewhere. Verify normalizes line endings on compare, and the new '*.verified.* text eol=lf' gitattribute keeps the snapshots LF in the repo, so the exact-output assertions are platform-independent without bespoke scaffolding.
ITaluone
left a comment
There was a problem hiding this comment.
Very nice work!
Thanks again
Closes #456.
Problem
GitHubActionsAttribute.GetSteps()yields a fixed, closed step sequence with no hook points and isprivate. No way to inject a marketplace action or custom shell step at a chosen position.Outcome
A build implements
IConfigureGitHubActions.ConfigureSteps(GitHubActionsStepPipeline)to inject typeduses:/run:steps at four named positions — compile-checked, no reflection.New public types:
GitHubActionsCustomStep,GitHubActionsStepPosition(PostCheckout/PreRun/PostRun/JobEnd),IConfigureGitHubActions,GitHubActionsStepPipeline.Notes
Uses/Run;WithrequiresUses;Shellonly on a run step.BuiltInStepsview — deliberately composes with the [GitHubActions] generator: emit CD pipelines (or a thin shell delegating to dotnet fallout Deploy) #336 job-DAG work rather than being reworked by it. Job-level concerns (needs:/outputs:/env gates) are out of scope for [GitHubActions] generator: emit CD pipelines (or a thin shell delegating to dotnet fallout Deploy) #336.[GitHubActionsInput]'s validation style.Acceptance criteria
uses:/run:at each of the four positions; multiple inserts at one position render in call order.ArgumentException.WorkflowName/Image(no per-step scoping arrays).Verification
Full
Fallout.Common.Specssuite: 114 passed, 7 skipped, 0 failed. Zero snapshot churn. Full-solution build clean.