Support dynamic branches in the inter-branch merge workflow - #17201
Support dynamic branches in the inter-branch merge workflow#17201PureWeen wants to merge 6 commits into
Conversation
Add optional `merge_from_branch` and `merge_to_branch` workflow_call inputs to `inter-branch-merge-base.yml`. Both are independently optional and callers that omit them keep the existing behaviour exactly: the source branch is inferred from `GITHUB_REF_NAME` and the target comes from the `MergeToBranch` value of the matching `github-merge-flow.jsonc` entry. The effective source branch is resolved once and used for both the configuration lookup and the merge itself, so a caller can flow a branch that did not trigger the workflow. The effective target prefers the input and otherwise falls back to the configured value, which lets a single source branch fan out to a target that is only known at run time. A matching configuration entry is still required, because that entry is what carries the merge policy (`ExtraSwitches`, `ResetToTargetPaths`); there is no configuration-free policy mode. The entry may now omit `MergeToBranch` when the caller supplies `merge_to_branch`, so `read-configuration.ps1` emits the policy outputs plus a new `policyFound` output whenever an entry matches. `configurationFound` keeps its original meaning of a complete, self-contained entry, so copies of the workflow pinned to an older ref are unaffected by the newer script, and the new workflow accepts either output so that an older `script_version` also keeps working. Merge execution is gated on a resolved target rather than on `configurationFound` alone. Caller supplied branch names are passed to the scripts through step environment variables rather than being interpolated into the PowerShell `run:` body, and are validated against a conservative allow-list plus `git check-ref-format` and an existence check on `origin`. `configuration_file_branch` and `configuration_file_path` were already interpolated into the same `run:` body and are moved to environment variables for the same reason. Related to #15586, which asks for multiple merge flows from one source branch. That issue describes a static fan-out that is known at configuration time; this change covers the dynamic case where the target is only known when the workflow runs. `inter-branch-merge.ps1` is unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc688ae1-1585-4842-b57a-b5c209f58a0e
The Azure DevOps PR pipeline excludes `.github/*` (see the path filters in azure-pipelines-pr.yml), so the inter-branch merge workflow and the scripts it drives had no automated validation at all. Add a Pester suite and a path filtered GitHub Actions workflow to run it, since Azure DevOps will not. The suite does not restate anything about the workflow. It reads the step bodies, the step `env:` mappings, the `working-directory` values and the `if:` gates out of the shipped YAML and then executes them, so the workflow and the tests cannot drift apart. Workflow expressions are resolved from a context that throws on anything it does not recognise, which means a rewired step fails the suite rather than quietly keeping its old value. `inter-branch-merge.ps1` is replaced by a stub generated from the real script's own parameter block, so the tests observe the exact command line the workflow builds and a command line the real script would reject fails here too. Covered: the configured target with no inputs, an overridden source, an overridden target, both overrides at once, an entry without `MergeToBranch` merged with an explicit target, preservation of `ExtraSwitches` and `ResetToTargetPaths` in every case, both directions of `script_version` skew, and twelve rejected branch inputs that each assert the reason they were rejected rather than only that something failed. Validated on Pester 5.7.1 and 6.0.1, against a CRLF checkout, and by mutating the production files nine ways; every mutation is caught. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc688ae1-1585-4842-b57a-b5c209f58a0e
`-MinimumVersion` let a runner image refresh or a PSGallery publish silently change which Pester CI ran against, and `-SkipPublisherCheck` waived module provenance to work around the Pester 3.4.0 that Windows ships in System32. Pin one exact, tested version instead. When the runner does not already have it, `Save-Module -RequiredVersion` places it in a job local directory under RUNNER_TEMP and the tests import it by manifest path. Saving rather than installing needs no publisher check waiver and cannot be shadowed by another Pester on the image, and the run then asserts the loaded version matches before running anything, so a mismatch fails the job rather than quietly testing against something else. Validated by running both step bodies verbatim, through the download path and the already-present path, and with actionlint. actionlint also caught that the `runner` context is not available in job level `env:`, so the directory is derived from RUNNER_TEMP inside the steps instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc688ae1-1585-4842-b57a-b5c209f58a0e
Two gaps in the workflow script tests. The Invoke-WebRequest shim ignored its arguments, so nothing observed the URL `read-configuration.ps1` builds. Mutating the branch, path, owner or repository the workflow wires into the lookup left the suite green. The shim now mirrors the parameters the real call passes, records the URI, and the tests assert the full raw.githubusercontent.com URL. Every component of that URL is a distinct value in the test context, so swapping any two of them is visible. All four mutations are now caught. The rejection reasons were matched against the child process console output, which PowerShell renders with ConciseView and wraps across gutter prefixed lines depending on console width and on the length of the branch value being reported. The child now records the raw exception message to a file and the assertions match that instead, so they no longer depend on how the message happened to be formatted. Validated with the pinned Pester 5.7.1, with Pester 6.0.1, by running the CI step bodies verbatim, with actionlint, and by re-running the earlier mutations to confirm they are all still caught. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc688ae1-1585-4842-b57a-b5c209f58a0e
Caller controlled values reach the scripts through step `env:` entries rather than being interpolated into a `run:` body, because a workflow expression is substituted into the script source before PowerShell ever parses it. Reverting that is value identical, so every behavioural test still passed with the inputs inlined again. Assert the shape directly: no `run:` body may interpolate `inputs.*`, the resolved branch outputs or `github.event`, and the caller controlled values must appear as step environment variables. Both revert mutations are now caught. Validated with the pinned Pester 5.7.1, with Pester 6.0.1, by running the CI step bodies verbatim, and with actionlint. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc688ae1-1585-4842-b57a-b5c209f58a0e
Two test quality fixes. The failure guards in the flow helper interpolated only the console log. Since the child records errors raw rather than printing them, that log is empty when a step fails, so a failure could be reported with a blank reason. Report the raw message and the log together. The structural guard was named and commented as though it covered all externally supplied data. It does not, and deliberately so: `ExtraSwitches` reaches the command line from the repository's own merge flow configuration file, which the existing design treats as trusted, and that predates this suite. Narrow the names and comments to what is actually asserted, the runtime workflow inputs and the source and target branches resolved from them, and state the exclusion explicitly. The assertions themselves are unchanged in coverage and now also pin the resolved source and target to step environment variables. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: fc688ae1-1585-4842-b57a-b5c209f58a0e
There was a problem hiding this comment.
Pull request overview
This PR extends the reusable inter-branch merge workflow to support runtime-selected source and/or target branches while preserving the existing default behavior for all current callers. It also adds a dedicated GitHub Actions workflow and a Pester test suite to validate the workflow’s PowerShell-backed logic despite .github/* being excluded from the Azure DevOps PR pipeline.
Changes:
- Add optional
merge_from_branchandmerge_to_branchworkflow_callinputs, including branch resolution + validation, and gate merge execution on a resolved target branch. - Update
read-configuration.ps1to support “policy-only” entries (policy present, optionalMergeToBranch), emitting a newpolicyFoundoutput while keepingconfigurationFoundsemantics for version-skew compatibility. - Add a Windows-runner GitHub Actions workflow that installs a pinned Pester version and runs a new Pester suite that executes workflow step bodies extracted from the shipped YAML.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/inter-branch-merge-base.yml |
Adds dynamic source/target inputs, resolves/validates branches, and adjusts gating so merges only run when configuration exists and a target is resolved. |
.github/workflows/scripts/read-configuration.ps1 |
Allows entries without MergeToBranch (policy-only), adds policyFound, and preserves old behavior via configurationFound meaning “self-contained entry”. |
.github/workflows/scripts/tests/InterBranchMerge.Tests.ps1 |
New Pester suite that parses and executes the workflow’s shipped step bodies to prevent test/workflow drift. |
.github/workflows/test-workflow-scripts.yml |
New CI workflow to run the Pester tests on windows-latest, pinning Pester to 5.7.1. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 0
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review: no actionable issues found.
Three independent reviewers examined the four authoritative changed files, including legacy callers, partial input combinations, workflow/script script_version skew, runtime-input handling, and the YAML-driven Pester harness. One potential native-command compatibility concern was challenged and discarded after official PowerShell 7.5/7.6 documentation and source confirmed its factual premise was incorrect.
The changed behavior is covered by the added 44-test Pester suite. The prior automated review also had no unresolved findings.
Methodology: 3 independent reviewers with adversarial consensus. Review event: COMMENT.
Adds two optional inputs to
.github/workflows/inter-branch-merge-base.ymlso a caller can decide the source and/or target branch at run time.What changes
Two new
workflow_callinputs, both defaulting to''and usable independently:merge_from_branchGITHUB_REF_NAME, as todaymerge_to_branchMergeToBranch, as todayThe effective source is resolved once and used for both the merge flow configuration lookup and the merge itself, so a caller can flow a branch that did not trigger the workflow. Existing callers that omit both inputs keep exactly the current behaviour.
A matching configuration entry is still required, because that entry is what carries the merge policy (
ExtraSwitches,ResetToTargetPaths). There is no configuration-free policy mode: a supplied target does not become a way to merge branches that carry no configured policy. The entry may now omitMergeToBranch, but only when the caller suppliesmerge_to_branch.Merge execution is gated on a resolved target rather than on
configurationFoundalone.Why
github-merge-flow.jsoncis keyed by source branch and stores a single staticMergeToBranch, so the target has to be known when the configuration is written. Two things fall outside that:GITHUB_REF_NAMEis the default branch rather than the branch that should actually flow. Without an input there is no way to say "run on a schedule, but merge this branch".Related to #15586, which asks for multiple merge flows from a single source branch. That issue describes static fan-out, where every target is known at configuration time and the workaround is calling the workflow N times. This change addresses the dynamic case, where the target is only known when the workflow runs, and as a side effect also unblocks the static case, since
merge_to_branchlets one configuration key drive several targets. I have not marked it as closing, because the issue may still warrant a first-class multi-target configuration shape.Safety
env:entries and read as$env:NAME. Nothing caller-controlled is interpolated into a PowerShellrun:body, since a workflow expression is substituted into the script source before PowerShell parses it. While in that step,configuration_file_branchandconfiguration_file_pathwere moved toenv:as well; they were already interpolated there...check, a 255 character cap,git check-ref-format, and an existence check onorigin. Values that are inferred fromGITHUB_REF_NAMEor read from configuration are not revalidated, so legacy behaviour is unchanged.ExtraSwitchesremains trusted and splatted, exactly as before. This PR does not change that design.Version skew
script_versionselects the checked-out scripts independently of the ref a caller pins the reusable workflow to, so the two can be different versions. Handled deliberately, with no new script parameter (an older pinned script would reject an unknown named parameter):read-configuration.ps1emits a newpolicyFoundoutput whenever an entry matches, and keepsconfigurationFoundmeaning "a complete, self-contained entry". A workflow pinned to an older ref therefore behaves exactly as before.script_versionalso keeps working.Reading an output an old script never emits is safe; passing an unknown parameter is not.
inter-branch-merge.ps1is unchanged.Testing
azure-pipelines-pr.ymlexcludes.github/*, so these files had no automated validation. This PR adds:.github/workflows/scripts/tests/InterBranchMerge.Tests.ps1— 44 Pester tests. The suite restates nothing about the workflow: it reads the step bodies, the stepenv:mappings, theworking-directoryvalues and theif:gates out of the shipped YAML and executes them, so the workflow and the tests cannot drift apart. Workflow expressions resolve through a context that throws on anything unrecognised.inter-branch-merge.ps1is replaced by a stub generated from the real script's own parameter block, so the tests observe the exact command line the workflow builds and a command line the real script would reject fails here too..github/workflows/test-workflow-scripts.yml— a path-filtered runner onwindows-latest, matching where the merge workflow runs, pinned to an exact Pester 5.7.1 so a runner image refresh or a PSGallery publish cannot silently change what CI runs.Covered: the configured target with no inputs, an overridden source, an overridden target, both overrides at once, an entry without
MergeToBranchmerged with an explicit target, preservation ofExtraSwitchesandResetToTargetPathsin every scenario, both directions ofscript_versionskew, the configuration URL that gets requested, and twelve rejected branch inputs that each assert the specific reason they were rejected.Validated with the pinned Pester 5.7.1 and with Pester 6.0.1, against a CRLF checkout, by running the CI step bodies verbatim, and with
actionlint(clean on both workflows). The suite was also mutation-tested: every deliberate regression introduced into the production files was caught.