Refactor release and auto-draft PR workflows for improved safety and maintainability#2
Open
Sebastian Bingel (sebingel) wants to merge 11 commits into
Open
Refactor release and auto-draft PR workflows for improved safety and maintainability#2Sebastian Bingel (sebingel) wants to merge 11 commits into
Sebastian Bingel (sebingel) wants to merge 11 commits into
Conversation
…jor tag
release.yaml had two problems. First, the file was named without the `_`
prefix required by the repository's naming convention for internal workflows
(only reusable workflows that external consumers call via `uses:` omit the
prefix). Second, the `update-major-tag` job used `git tag --force` and
`git push --force`, which required a full checkout with `fetch-depth: 0`
and `persist-credentials: true`. On top of that, `${{ github.ref_name }}`
was interpolated directly into the `run:` string, creating a shell injection
vector: a maliciously named tag could execute arbitrary commands during
the release workflow.
Changes:
- Rename `.github/workflows/release.yaml` -> `_release.yaml`
Aligns with the convention that `_`-prefixed workflow files are internal.
References in `README.md` and `.github/copilot-instructions.md` updated.
- Replace bash `git tag --force` + `git push --force` with `gh api` REST calls
(`_release.yaml`, `update-major-tag` job; new `update-major-tag.ps1`)
Eliminates the need for `fetch-depth: 0` and `persist-credentials: true`.
The checkout is now sparse (`.github/scripts/_release` only) with
`persist-credentials: false`. Annotated tag objects are peeled to their
commit SHA via the git tags API before writing the major-version tag.
- Fix shell injection in `update-major-tag` step (`_release.yaml:71`)
`${{ github.ref_name }}` is now set as `RELEASE_TAG` in `env:` (safe) and
read via `$env:RELEASE_TAG` at PowerShell runtime instead of being
interpolated into the `run:` string at template-expansion time.
- Extract inline script to `.github/scripts/_release/update-major-tag.ps1`
Adds `[CmdletBinding()]`, `param([string] $Tag)`, comment-based help, and
section headers. The 404 / non-404 distinction in the major-tag existence
check uses `$LASTEXITCODE` + string-cast `-notmatch 'Not Found'` instead of
a bare `try/catch` that would swallow auth failures and 5xx errors silently.
- Add `.github/scripts/_release/ghmock.sh`
Mock `gh` binary driven by env vars (`MOCK_TAG_TYPE`, `MOCK_MAJOR_EXISTS`,
`MOCK_MAJOR_TAG_ERROR`, `MOCK_TAG_SHA`, `MOCK_COMMIT_SHA`). Uses
`$GITHUB_PATH` to persist the mock PATH across steps.
- Add `.github/workflows/_test-release.yaml` with four test jobs
Covers: non-SemVer skip, lightweight-tag POST, annotated-tag peel + PATCH,
and unexpected API error propagation. The error-propagation test re-exposes
the script exit code via `exit $LASTEXITCODE` after `*>&1 | Tee-Object |
Out-Null` to prevent `Out-Null` from masking non-zero exits.
- Add `docs/release.md`
Documents the workflow, trigger, pre-release handling, and changelog labels.
The major-tag roll now requires no git credentials and no local repository
state beyond the script file itself. The shell injection surface is removed.
All three code paths of update-major-tag.ps1 (lightweight, annotated, error)
are covered by integration tests with a mocked gh binary.
…eouts The auto-draft-pr.yaml workflow embedded its entire PowerShell implementation as a 66-line inline `run:` block. To allow the integration tests to call the same code, a byte-identical copy had to be maintained in test-auto-draft-pr/run-impl.ps1 alongside a "drift detection" CI job that diffed the two with yq+sed. Any edit to the workflow required a manual sync to the mirror file in the same commit; drift went silently unnoticed between commits until CI caught it. Both ghmock setup scripts were also bash despite the repository's PowerShell-preferred convention, and no workflow job had a timeout-minutes guard. Changes: - Add .github/scripts/auto-draft-pr/open-draft-pr.ps1 The extracted implementation with [CmdletBinding()], comment-based help, and the same section-header style used by update-major-tag.ps1. All env vars are read from the process environment (no parameter injection risk). - Update auto-draft-pr.yaml (steps, timeout) Replaces the 66-line inline run-block with a sparse checkout of .github/scripts/auto-draft-pr followed by a single-line script call. Adds timeout-minutes: 5 to the draft-pr job. - Delete test-auto-draft-pr/run-impl.ps1 and test-auto-draft-pr/ghmock.sh The MIRROR pattern and its drift-detection job are no longer needed now that the workflow and the tests both call the same .ps1 file. - Add .github/scripts/auto-draft-pr/ghmock.ps1 (replaces bash ghmock) PowerShell setup script that writes the mock gh bash binary to $HOME/mock-bin and registers it via $GITHUB_PATH. The mock binary itself stays bash (POSIX executable without extension), embedded as a single-quoted PowerShell here-string so $ variables are not expanded. - Add .github/scripts/_release/ghmock.ps1 (replaces _release/ghmock.sh) Same pattern for the release test suite. Also fixes the /tmp/gh-mock path from the original bash script to $HOME/gh-mock, consistent with the auto-draft-pr mock. - Update _test-auto-draft-pr.yaml Removes the drift-detection job entirely. All four test jobs now sparse- checkout .github/scripts/auto-draft-pr, run ghmock.ps1 via shell: pwsh, and call open-draft-pr.ps1 directly. Adds timeout-minutes: 10 to each. - Update _test-release.yaml, _release.yaml, _lint.yaml (timeout only) Adds timeout-minutes: 10 to all remaining jobs (8 jobs across 3 files) that had no hang protection. - Update copilot-instructions.md Removes "full PowerShell logic inline" language; points to the new open-draft-pr.ps1 location. After this change the single source of truth for the auto-draft-pr implementation is open-draft-pr.ps1. The MIRROR pattern and its drift-detection overhead are gone. Every workflow job is now protected by a timeout. No bash setup scripts remain in the repository.
…ushes The test job "Unexpected API error => non-zero exit" in _test-release.yaml was failing with `grep -qi "Unexpected error" output.txt` returning exit 1. The output.txt file existed but did not contain the "Unexpected error..." message, even though GitHub Actions showed it in the runner log. The root cause: $ErrorActionPreference = 'Stop' is set at the top of update-major-tag.ps1 (line 29). When Write-Error is called without an explicit -ErrorAction override, Stop converts it into a terminating exception immediately, before exit 1 can run. In the test pipeline update-major-tag.ps1 ... *>&1 | Tee-Object -FilePath output.txt | Out-Null the terminating exception tears down the pipeline before Tee-Object has a chance to flush the redirected stream-2 content to disk. The message appears in the GitHub Actions runner log (via the exception handler) but not in output.txt, so the grep assertion always fails. Changes: - Add -ErrorAction Continue to Write-Error call (.github/scripts/_release/update-major-tag.ps1:64) Overrides the ambient $ErrorActionPreference = 'Stop' for this one call. Write-Error writes the message to the error stream (stream 2) as a non-terminating error, then execution continues to exit 1. The *>&1 redirect in the test pipeline captures stream 2, Tee-Object writes it to output.txt, and the grep assertion finds "Unexpected error". The default error-reporting behavior of the script in production is unchanged: any unexpected API error still causes a non-zero exit. Only the mechanism differs (explicit exit 1 instead of exception propagation), which is more predictable and testable.
There was a problem hiding this comment.
Pull request overview
This PR refactors the internal release automation and the public auto-draft-pr reusable workflow by moving previously-inline logic into dedicated PowerShell scripts, adding PowerShell-based gh CLI mocks for integration tests, and tightening workflow safety (sparse checkouts, no persisted credentials, job timeouts).
Changes:
- Refactors release major-tag rolling into
.github/scripts/_release/update-major-tag.ps1and updates_release.yamlto use the GitHub REST API viagh api. - Extracts auto-draft PR creation logic into
.github/scripts/auto-draft-pr/open-draft-pr.ps1and updates the reusable workflow + test workflow to use the script and PowerShell-based mocks. - Adds new internal integration tests (
_test-release.yaml) and introduces job timeouts across lint/release/test workflows; updates docs/README/instructions accordingly.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Updates release workflow reference to _release.yaml. |
| docs/release.md | Adds documentation for the internal _release.yaml workflow and major-tag behavior. |
| .github/workflows/auto-draft-pr.yaml | Switches reusable workflow from inline implementation to checked-in PowerShell script and adds timeout/sparse checkout. |
| .github/workflows/_test-release.yaml | Adds integration tests for update-major-tag.ps1 using a mocked gh CLI. |
| .github/workflows/_test-auto-draft-pr.yaml | Updates tests to use the new PowerShell implementation and mock; removes drift-detection job. |
| .github/workflows/_release.yaml | Refactors major-tag rolling to call update-major-tag.ps1 and adds timeouts/sparse checkout. |
| .github/workflows/_lint.yaml | Adds job timeouts. |
| .github/scripts/test-auto-draft-pr/ghmock.sh | Removes the old bash-based gh mock (replaced by PowerShell-based setup). |
| .github/scripts/auto-draft-pr/open-draft-pr.ps1 | New/updated extracted implementation for opening draft PRs. |
| .github/scripts/auto-draft-pr/ghmock.ps1 | Adds PowerShell installer for a mock gh binary used by auto-draft-pr tests. |
| .github/scripts/_release/update-major-tag.ps1 | Adds PowerShell script to roll major tags via REST API with annotated-tag peeling and error handling. |
| .github/scripts/_release/ghmock.ps1 | Adds PowerShell installer for a mock gh binary used by release tests. |
| .github/copilot-instructions.md | Updates repository guidance to reflect the new script-based implementations. |
Reusable workflows run in the caller's repository context, meaning
GITHUB_REPOSITORY resolves to the caller's repo. The actions/checkout
step was missing an explicit `repository` and `ref`, so it would attempt
a sparse-checkout of .github/scripts/auto-draft-pr from the caller's
repository -- which does not contain those files. The workflow would fail
with "path not found" on every invocation.
Changes:
- .github/workflows/auto-draft-pr.yaml:43-47
Added `repository: jtl-software/actions` so the checkout always targets
this repo regardless of which caller triggered the workflow.
- .github/workflows/auto-draft-pr.yaml:44
Added `ref: ${{ job.workflow_sha }}` to pin the checkout to the exact
commit of the reusable workflow being executed. `job.workflow_sha`
resolves to the SHA of the file defining the current job (the callee);
`github.workflow_sha` would resolve to the caller's workflow file SHA,
which is an unrelated and incorrect reference.
The open-draft-pr.ps1 script is now reliably fetched from
jtl-software/actions at the same SHA that the workflow was called at,
matching the pattern already used in _release.yaml.
The previous description said "Tags containing a hyphen (pre-releases) are silently skipped." This was wrong on two counts: the script writes a log message and exits 0 (not silent), and the regex `^v(\d+)\.\d+\.\d+$` rejects any tag that is not strict SemVer -- not just hyphenated ones (e.g. a plain "latest" tag would also be rejected). Changes: - .github/scripts/_release/update-major-tag.ps1:15-18 Replaced the one-liner with a two-sentence description that reflects the actual exit-0-with-log behavior, names the upstream workflow-level `if:` guard that handles the common pre-release case, and clarifies that this check is a general safety net for direct script invocations, not a hyphen-specific filter. Readers inspecting the script directly now get an accurate picture of when and why the early-exit path is taken without needing to cross-reference the workflow YAML.
job.workflow_sha does not exist in the GitHub Actions job context
(actionlint: "property workflow_sha is not defined in object type").
github.workflow_sha is the correct variable: in a reusable workflow
triggered via workflow_call, GitHub populates github.workflow_sha with
the SHA of the called (reusable) workflow file itself, not the caller's
workflow SHA.
Changes:
- .github/workflows/auto-draft-pr.yaml:47
Replace `ref: ${{ job.workflow_sha }}` with `ref: ${{ github.workflow_sha }}`
so actionlint passes and the checkout resolves to the correct SHA.
The test-api-error-propagates job's "Run and expect failure" step ended with `exit $LASTEXITCODE` after the Tee-Object pipeline, along with a comment claiming this re-propagates the script's exit code to prevent Out-Null from masking failures. Both the code and the comment were incorrect. update-major-tag.ps1 uses `exit 1` (line 67) on the API error path. In PowerShell, `exit` terminates the entire pwsh process immediately; the outer run-block's `exit $LASTEXITCODE` line therefore never executes -- it is unreachable dead code. The step's non-zero exit code is already provided by the script's own `exit 1`, and `continue-on-error: true` captures the resulting `failure` outcome regardless. Additionally, `$LASTEXITCODE` reflects native command exit codes, not PowerShell script exit codes, so the line would produce the wrong value anyway if it were reachable. Changes: - .github/workflows/_test-release.yaml:150-152 Remove the three-line dead-code block (comment + `exit $LASTEXITCODE`) from the "Run and expect failure" step of test-api-error-propagates. The test's correctness is unaffected: the process exits with code 1 via the script's `exit 1`, `continue-on-error: true` marks the step as `failure`, the assertion checks `steps.run.outcome == "failure"`, and the `Write-Error -ErrorAction Continue` fix in the script ensures the error message reaches output.txt via Tee-Object before the process terminates.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
.github/scripts/auto-draft-pr/open-draft-pr.ps1:14
- The comment-based help says the required environment variables are "set automatically by GitHub Actions", but most of these (COMMIT_MSG, BRANCH, DEFAULT_BRANCH, ACTOR, REPO) are actually populated by the calling workflow via
env:. Consider rewording this section to avoid implying these env vars exist automatically when running the script outside that workflow context.
.github/scripts/auto-draft-pr/open-draft-pr.ps1:23 - This comment mentions native-command failures from "gh, jq", but the script no longer invokes
jqdirectly (onlygh). Updating the comment would prevent confusion about runtime dependencies.
The testing section's last sentence described a drift-detection CI job that kept the inline `run:` block in `auto-draft-pr.yaml` byte-identical to a mirror script at `.github/scripts/test-auto-draft-pr/run-impl.ps1`. Both artefacts no longer exist: the inline block was extracted to `open-draft-pr.ps1`, the mirror file and its drift-detection job were deleted as part of that refactor. The outdated sentence was therefore describing infrastructure that has been gone since the refactor commit, and would mislead anyone reading the docs about how the tests work. Changes: - docs/auto-draft-pr.md:62 Replace the drift-detection sentence with an accurate description: `ghmock.ps1` sets up the mock gh binary, and the tests invoke `open-draft-pr.ps1` directly -- the same file the reusable workflow calls in production. Explicitly states that no mirror file or drift-detection job exists. The testing section now correctly reflects the current setup, where the single source of truth for the implementation is `open-draft-pr.ps1` and the tests exercise it without any synchronization overhead.
…nt and job name The header comment on line 9 and the test-title-fallback job name on line 135 both described the fallback PR title format incorrectly. The comment used `Draft:<branch>` (no space after the colon) and the job name used `Draft:-branch`, neither of which matches the actual output produced by `open-draft-pr.ps1` line 53: `$title = "Draft: $($env:BRANCH)"`. The real format is always `Draft: ` followed by the branch name, with a space after the colon. The mismatch made the test workflow misleading when reading the header or inspecting job results in the GitHub Actions UI. Changes: - .github/workflows/_test-auto-draft-pr.yaml:9 Replace `Draft:<branch>` with `Draft: <branch>` in the file-level header comment so it accurately reflects the script's string format. - .github/workflows/_test-auto-draft-pr.yaml:135 Replace job display name `No Titel-prefix => Draft:-branch fallback` with `No Titel-prefix => Draft: <branch> fallback` so the name shown in the GitHub Actions UI matches the actual title the script produces (e.g. "Draft: feature/foo"), consistent with the grep assertion at line 165: `--title Draft: feature/foo`. Both the comment and the job name now correctly reflect the space-after- colon format confirmed by the script source and the test assertion.
YAML interprets a bare colon-space (`: `) in an unquoted scalar as a mapping separator, causing actionlint to report "mapping values are not allowed in this context [syntax-check]" at line 135. The job name "No Titel-prefix => Draft: <branch> fallback" contains `Draft: ` which triggered this error. Changes: - .github/workflows/_test-auto-draft-pr.yaml:135 Wrap the job name in double quotes so YAML parses the colon-space as a literal character rather than a mapping indicator.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
.github/scripts/auto-draft-pr/open-draft-pr.ps1:16
- The comment says these environment variables are "set automatically by GitHub Actions", but most of them (e.g. COMMIT_MSG, BRANCH, DEFAULT_BRANCH, ACTOR, REPO) are only present because the workflow step explicitly sets them. Consider rewording to "must be provided by the calling workflow" (or similar) to avoid confusing anyone running the script locally/tests without that env setup.
.github/scripts/auto-draft-pr/open-draft-pr.ps1:23 - This comment mentions "native commands (gh, jq)", but the script doesn't invoke a separate
jqbinary (it usesgh api --jq). To keep the rationale accurate, consider removingjqfrom the comment (or explain that--jqis handled bygh).
The .DESCRIPTION block grouped GH_TOKEN and GITHUB_REPOSITORY under the
heading "set automatically by GitHub Actions". This is only true for
GITHUB_REPOSITORY; GH_TOKEN is not a standard runner-injected variable.
The GitHub Actions runner injects GITHUB_TOKEN, but the gh CLI reads
GH_TOKEN -- a different name. Any workflow step calling this script must
explicitly map the token via `env: GH_TOKEN: ${{ github.token }}`, as
_release.yaml already does. A contributor reading the doc in isolation
could incorrectly assume the variable is available without any wiring,
leading to auth failures when invoking the script directly.
Changes:
- .github/scripts/_release/update-major-tag.ps1:11-17
Remove the "(set automatically by GitHub Actions)" qualifier from the
shared heading and add per-variable clarifications: GH_TOKEN documents
the required explicit env: mapping and the reason (gh CLI convention,
not a runner default); GITHUB_REPOSITORY notes it is populated
automatically, preserving the accurate part of the original text.
Contributors can now distinguish at a glance which variable needs wiring
in a workflow step and which is free. The script's runtime behavior is
unchanged; only the documentation is corrected.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
.github/scripts/auto-draft-pr/open-draft-pr.ps1:16
- The comment-based help says the required environment variables are "set automatically by GitHub Actions", but at least
GH_TOKENis not populated by the runner (it’s explicitly provided by the workflow step). Consider rewording this section to distinguish variables provided by the runner vs. variables that must be passed in via the workflowenv:block, to avoid misleading future callers/test authors.
Tobias Lengsholz (tobilen)
requested changes
May 12, 2026
Tobias Lengsholz (tobilen)
left a comment
Tobias Lengsholz (tobilen)
left a comment
There was a problem hiding this comment.
how come we're using powershell instead of bash? another language on the ever growing stack of languages. Also, I can't really review the powershell code as I am not familiar enough with the language myself.
Looks AI generated to me, would be pretty easy to ask AI to rewrite it to bash, no? 😉
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.
This pull request refactors and modernizes the release and auto-draft-PR workflows, moving implementation logic into dedicated, testable PowerShell scripts and improving testability and maintainability. It replaces inline and bash-based scripts with PowerShell equivalents, introduces robust mocking for the
ghCLI in tests, and updates workflows to use these scripts. Additionally, it streamlines the major tag update process to use the GitHub REST API, ensures security best practices, and adds timeouts for reliability.Release workflow modernization and security:
The release workflow file is renamed to
_release.yaml, and the major-version tag update logic is moved from inline bash to the new PowerShell script.github/scripts/_release/update-major-tag.ps1, which uses the GitHub REST API for safer and more reliable tag management. The workflow now checks out only the required scripts and avoids fetching git history or credentials. [1] [2] [3]A PowerShell-based mock for the
ghCLI is introduced for release workflow tests, allowing simulation of various API responses and error conditions.Auto-draft-PR workflow refactor and testability:
The core implementation for opening draft PRs is moved from an inline script to
.github/scripts/auto-draft-pr/open-draft-pr.ps1(renamed fromtest-auto-draft-pr/run-impl.ps1), making it easier to test and maintain. The documentation and workflow references are updated accordingly. [1] [2] [3]The test workflow
_test-auto-draft-pr.yamlis updated to use the new PowerShell-based implementation and mock, removing the bash-based mock and drift-detection logic. This streamlines testing and ensures consistency with the production workflow. [1] [2] [3] [4] [5] [6] [7]General workflow improvements:
Documentation updates:
Summary of most important changes:
Release workflow refactor and security:
.github/scripts/_release/update-major-tag.ps1, using the GitHub REST API and improving security by checking out only scripts and not credentials or git history. [1] [2] [3]ghCLI for release workflow tests.Auto-draft-PR workflow refactor and testability:
.github/scripts/auto-draft-pr/open-draft-pr.ps1and updated documentation and references to use this script. [1] [2] [3]General improvements:
Documentation: