CNTRLPLANE-4013: add GitHub Actions workflow rehearsal script - #9253
CNTRLPLANE-4013: add GitHub Actions workflow rehearsal script#9253celebdor wants to merge 1 commit into
Conversation
Adds contrib/ci/gha-rehearse.sh to enable rehearsing new GitHub Actions workflows from PR branches before merging. GitHub Actions workflow_dispatch requires the workflow file to exist on the default branch to appear in the UI. This script pushes minimal stubs to the default branch so an admin can select the PR branch and run the real workflow code, then cleans up the stub commit afterwards. Ref: CNTRLPLANE-4013 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@celebdor: This pull request references CNTRLPLANE-4013 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
📝 WalkthroughWalkthroughAdds Sequence Diagram(s)sequenceDiagram
participant Operator
participant gha-rehearse.sh
participant GitRepository
participant GitHubActions
Operator->>gha-rehearse.sh: Run setup
gha-rehearse.sh->>GitRepository: Find changed workflows
gha-rehearse.sh->>GitRepository: Commit and push stubs
Operator->>GitHubActions: Dispatch workflows on the PR branch
Operator->>gha-rehearse.sh: Run cleanup
gha-rehearse.sh->>GitRepository: Validate rehearsal state
gha-rehearse.sh->>GitRepository: Reset the default branch
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (10 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: celebdor The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
contrib/ci/gha-rehearse.sh (2)
116-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the workflow names instead of re-parsing the generated stubs.
Lines 79-80 already extract each workflow name. Line 118 parses the same value back out of the file that Line 82 just wrote. The two parsers must stay identical, and they already share the same portability defect.
Store the names in a parallel array during the generation loop.
♻️ Proposed refactor
+ local -a stub_names=() for wf in "${new_workflows[@]}"; do local name name="$(git show "FETCH_HEAD:${wf}" 2>/dev/null \ | grep -m1 '^name:' | sed 's/^name:[[:space:]]*//' || echo "${wf##*/}")" + stub_names+=("$name") mkdir -p "${tmpdir}/$(dirname "$wf")" generate_stub "$name" > "${tmpdir}/${wf}" doneecho "To rehearse, run:" - for wf in "${new_workflows[@]}"; do - local stub_name - stub_name="$(grep -m1 '^name:' "${tmpdir}/${wf}" | sed 's/^name:\s*//')" - echo " gh workflow run '${stub_name}' --repo ${REPO} --ref ${head_branch}" + for stub_name in "${stub_names[@]}"; do + echo " gh workflow run '${stub_name}' --repo ${REPO} --ref ${head_branch}" done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contrib/ci/gha-rehearse.sh` around lines 116 - 120, Update the workflow generation loop to store each extracted workflow name in a parallel array alongside new_workflows, then use the corresponding stored name when emitting the gh workflow run command. Remove the grep/sed re-parsing in the final loop and preserve the existing name extraction logic used during generation.
110-110: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a confirmation prompt before the script writes to the default branch.
This line pushes to the default branch of a shared repository without asking the user. A mistyped pull request number, or a run against the wrong repository, publishes stub workflows to
mainfor every collaborator. Recovery requires thecleanupcommand and depends on the marker file surviving.The push also requires permission to bypass branch protection. The header comment at Lines 9-16 lists
gh CLI, gitas the only requirements. Add the admin push requirement there.Consider a prompt that the user can skip with an environment variable, so automated tests stay non-interactive.
♻️ Proposed change
+ if [[ "${GHA_REHEARSE_ASSUME_YES:-}" != "1" ]]; then + read -r -p "Push ${`#new_workflows`[@]} stub(s) to ${REPO}:${DEFAULT_BRANCH}? [y/N] " reply + [[ "$reply" == [yY] ]] || die "aborted by user" + fi git push origin "${stub_commit}:refs/heads/${DEFAULT_BRANCH}"And in the header comment:
-# Requirements: gh CLI, git +# Requirements: gh CLI, git, and permission to push directly to the default +# branch (bypassing branch protection).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@contrib/ci/gha-rehearse.sh` at line 110, Update the push flow in gha-rehearse.sh to require explicit user confirmation before pushing stub_commit to DEFAULT_BRANCH, while allowing non-interactive runs to bypass the prompt through an environment variable. Document the bypass variable and the required permission to bypass branch protection in the script’s header requirements alongside the existing gh CLI and git prerequisites.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contrib/ci/gha-rehearse.sh`:
- Line 80: In both name-extraction sites using sed in
contrib/ci/gha-rehearse.sh#L80-L80 and contrib/ci/gha-rehearse.sh#L118-L118,
replace the GNU-specific \s pattern with the POSIX [[:space:]] class so
whitespace is stripped correctly on BSD/macOS sed.
- Around line 55-60: Move the fetch of origin/${DEFAULT_BRANCH} in the rehearse
script to before the new_workflows existence-check loop, ensuring git cat-file
evaluates the current remote-tracking tree. Remove the later redundant fetch
while preserving the existing workflow generation and push flow.
- Around line 138-139: Update the cleanup push in the rehearsal script to use
--force-with-lease, pinning the lease to the default branch tip fetched at line
126, while still pushing parent to refs/heads/${DEFAULT_BRANCH}.
- Around line 72-73: Update the EXIT trap in cmd_setup to expand tmpdir when the
trap is defined rather than when it executes, while preserving safe cleanup of
the temporary directory under set -u.
- Around line 20-21: Move the die function above the repository metadata
initialization, then defer gh repo view calls from the top-level scope into the
command execution path so usage errors avoid network calls. Replace readonly
command substitutions with separate assignments for REPO and DEFAULT_BRANCH,
validate that both are non-empty, call die with a clear error when either lookup
fails, and declare the validated values readonly afterward.
- Around line 43-49: Update the workflow-file discovery in the rehearse script
to retrieve all pull-request files, replacing the non-paginated gh pr view files
query with gh api --paginate against the pull request files endpoint and
extracting each filename. Preserve the existing workflow YAML filtering and
workflow_files population, and continue deriving head_branch from the
pull-request metadata.
---
Nitpick comments:
In `@contrib/ci/gha-rehearse.sh`:
- Around line 116-120: Update the workflow generation loop to store each
extracted workflow name in a parallel array alongside new_workflows, then use
the corresponding stored name when emitting the gh workflow run command. Remove
the grep/sed re-parsing in the final loop and preserve the existing name
extraction logic used during generation.
- Line 110: Update the push flow in gha-rehearse.sh to require explicit user
confirmation before pushing stub_commit to DEFAULT_BRANCH, while allowing
non-interactive runs to bypass the prompt through an environment variable.
Document the bypass variable and the required permission to bypass branch
protection in the script’s header requirements alongside the existing gh CLI and
git prerequisites.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 976153a3-0c95-4b81-8880-9a19c61f2d12
📒 Files selected for processing (1)
contrib/ci/gha-rehearse.sh
| readonly REPO="${GHA_REHEARSE_REPO:-$(gh repo view --json nameWithOwner -q .nameWithOwner)}" | ||
| readonly DEFAULT_BRANCH="${GHA_REHEARSE_DEFAULT_BRANCH:-$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name)}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard against silent failures of gh repo view.
set -e does not abort on a failed command substitution inside a readonly declaration. The declaration builtin returns 0, so its own status masks the failure. If gh is unauthenticated, if the network fails, or if the current directory is not a GitHub repository, REPO and DEFAULT_BRANCH become empty strings.
An empty DEFAULT_BRANCH propagates into git push origin "${stub_commit}:refs/heads/" at Line 110 and into origin/ refs elsewhere. Separate the assignment from readonly and validate both values.
These two commands also run on every invocation, including the usage error path at Line 147. Deferring them into the command functions removes two network calls from the failure path.
🛡️ Proposed fix
-readonly REPO="${GHA_REHEARSE_REPO:-$(gh repo view --json nameWithOwner -q .nameWithOwner)}"
-readonly DEFAULT_BRANCH="${GHA_REHEARSE_DEFAULT_BRANCH:-$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name)}"
+REPO="${GHA_REHEARSE_REPO:-$(gh repo view --json nameWithOwner -q .nameWithOwner)}" \
+ || die "cannot determine repository; is 'gh' authenticated?"
+DEFAULT_BRANCH="${GHA_REHEARSE_DEFAULT_BRANCH:-$(gh repo view --json defaultBranchRef -q .defaultBranchRef.name)}" \
+ || die "cannot determine default branch; is 'gh' authenticated?"
+[[ -n "$REPO" ]] || die "repository name is empty"
+[[ -n "$DEFAULT_BRANCH" ]] || die "default branch name is empty"
+readonly REPO DEFAULT_BRANCH
readonly MARKER_FILE=".github/.gha-rehearse-marker"Note: die must be defined before this block for the fix to work. Move the die definition above the assignments.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contrib/ci/gha-rehearse.sh` around lines 20 - 21, Move the die function above
the repository metadata initialization, then defer gh repo view calls from the
top-level scope into the command execution path so usage errors avoid network
calls. Replace readonly command substitutions with separate assignments for REPO
and DEFAULT_BRANCH, validate that both are non-empty, call die with a clear
error when either lookup fails, and declare the validated values readonly
afterward.
| local -r pr_json="$(gh pr view "$pr_number" --repo "$REPO" --json headRefName,files)" | ||
| local -r head_branch="$(echo "$pr_json" | jq -r '.headRefName')" | ||
|
|
||
| local -a workflow_files=() | ||
| while IFS= read -r path; do | ||
| workflow_files+=("$path") | ||
| done < <(echo "$pr_json" | jq -r '.files[].path' | grep '^\.github/workflows/.*\.ya\?ml$') |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
gh pr view --json files 100 file limit truncated
💡 Result:
The command gh pr view --json files historically suffers from silent truncation, limiting results to only the first 100 changed files without warning [1][2]. This occurs because the underlying GitHub CLI implementation previously lacked pagination support for the files field [1][3]. If you need to retrieve a complete list of files for a pull request that has more than 100 changed files, you should use the GitHub CLI's API interface, which supports pagination via the --paginate flag [4]. The recommended alternative approach is to use the REST API endpoint for PR files: gh api --paginate repos/{owner}/{repo}/pulls/{pull_number}/files --jq '.[].filename' Alternatively, you can use the GraphQL API if you require specific file data, ensuring you include pageInfo to handle the pagination: gh api --paginate graphql -f query=' query ($owner: String!, $repo: String!, $pr: Int!, $endCursor: String) { repository(owner: $owner, name: $repo) { pullRequest(number: $pr) { files(first: 100, after: $endCursor) { nodes { path } pageInfo { hasNextPage, endCursor } } } } }' -F owner='{owner}' -F repo='{repo}' -F pr={pull_number} --jq '.data.repository.pullRequest.files.nodes[].path' Please note that efforts to implement automatic pagination directly within the gh pr view command have been documented and addressed in the GitHub CLI repository [3], so you may want to check if a recent version of the CLI has resolved this limitation in your specific environment [1][3].
Citations:
- 1:
gh pr view --json filesand--json commitssilently truncate at 100 items cli/cli#13338 - 2: The
pr viewcommand with the--json filesflag omits some of the files changed cli/cli#9916 - 3: Paginate files and commits in
gh pr view --jsoncli/cli#13340 - 4: Add option for gh pr diff to list the filenames that has changes on the PR cli/cli#1393
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- script context ---'
sed -n '1,100p' contrib/ci/gha-rehearse.sh
printf '%s\n' '--- gh CLI references and version constraints ---'
rg -n --hidden -S 'gh (pr view|api)|GitHub CLI|gh version' .github contrib README.md 2>/dev/null | head -200 || trueRepository: openshift/hypershift
Length of output: 3798
🌐 Web query:
site:cli.github.com/manual gh pr view --json files pagination changed files
💡 Result:
The gh pr view command does not support manual pagination for the files field when using the --json flag [1]. When you request the files field via gh pr view --json files, the GitHub CLI fetches file data associated with the pull request from the GitHub API [1][2]. Because gh interacts directly with the API, it is subject to the default limits and behaviors of the underlying GitHub GraphQL API, which typically truncates or limits the number of files returned in a single request (often to 100) [1][2]. If you need to retrieve a list of all changed files, particularly for pull requests with many changed files, consider these alternatives: 1. Use gh pr diff --name-only to list all file paths changed in the pull request. This command is specifically designed to output file names and handles larger sets of files more effectively than the files JSON field [3]. 2. If you must use JSON, verify the total number of changed files using the changedFiles field [1][4], and if the number exceeds the limit of the files field, you may need to use a script that interacts with the GitHub REST API (gh api) to paginate through the pull request files endpoint manually. In summary, the files field in gh pr view is not a paginated field that you can control via CLI flags [1][2]. For comprehensive lists of changed files, gh pr diff --name-only is the recommended approach [3].
Citations:
- 1: https://cli.github.com/manual/gh_pr_view
- 2: https://cli.github.com/manual/gh_help_formatting
- 3: https://cli.github.com/manual/gh_pr_diff
- 4: https://cli.github.com/manual/gh_pr_status
🌐 Web query:
site:github.com/cli/cli gh pr view files pagination 100 issue pull request
💡 Result:
The command gh pr view --json files has historically been subject to a silent truncation limit, returning a maximum of 100 files for pull requests that contain more [1][2][3]. This limitation occurs because the underlying GraphQL query previously lacked pagination support for the files field [1][4]. As of August 2026, work has been undertaken to address this issue by adding proper pagination to the files and commits fields in gh pr view [1][4]. Specifically, pull requests addressing this issue have implemented pageInfo handling to fetch all files regardless of the count, matching the pattern used for other paginated resources like reviews and comments [1][4]. If you are using a version of the GitHub CLI that does not yet include these fixes, or if you need to handle extremely large datasets in automated workflows, the recommended workaround is to use gh api graphql to construct a custom paginated query [5][6]. You can use the --paginate flag with gh api to automatically follow pageInfo and retrieve all results [5][6]: gh api graphql -f query=' query($owner: String!, $repo: String!, $pr: Int!, $endCursor: String) { repository(owner: $owner, name: $repo) { pullRequest(number: $pr) { files(first: 100, after: $endCursor) { pageInfo { hasNextPage, endCursor } nodes { path } } } } }' -F owner='OWNER' -F repo='REPO' -F pr=123 --paginate --jq '.data.repository.pullRequest.files.nodes.[].path' This approach allows you to bypass the 100-item restriction by explicitly iterating through the file list [5][6].
Citations:
- 1:
gh pr view --json filesand--json commitssilently truncate at 100 items cli/cli#13338 - 2: The
pr viewcommand with the--json filesflag omits some of the files changed cli/cli#9916 - 3: It seems that`gh pr view --json files` files count is limited but not documented cli/cli#6930
- 4: Paginate files and commits in
gh pr view --jsoncli/cli#13340 - 5: Remove restrictions of
gh pr view --json filescli/cli#5368 - 6: Remove restrictions of
gh pr view --json filescli/cli#5368
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- local script context ---'
sed -n '35,75p' contrib/ci/gha-rehearse.sh
printf '%s\n' '--- installed CLI, if present ---'
if command -v gh >/dev/null 2>&1; then
gh version
else
echo 'gh is not installed'
fi
printf '%s\n' '--- GitHub CLI issue and pull request metadata ---'
python3 - <<'PY'
import json, urllib.request
for path in (
"repos/cli/cli/issues/13338",
"repos/cli/cli/issues/9916",
"repos/cli/cli/pulls/13340",
):
req = urllib.request.Request(
"https://api.github.com/" + path,
headers={"Accept": "application/vnd.github+json", "User-Agent": "review-verifier"},
)
try:
with urllib.request.urlopen(req, timeout=15) as response:
data = json.load(response)
print(json.dumps({
"path": path,
"state": data.get("state"),
"title": data.get("title"),
"merged": data.get("merged"),
"merged_at": data.get("merged_at"),
"closed_at": data.get("closed_at"),
"html_url": data.get("html_url"),
}, sort_keys=True))
except Exception as exc:
print(json.dumps({"path": path, "error": str(exc)}))
PYRepository: openshift/hypershift
Length of output: 2268
🌐 Web query:
GitHub CLI current release gh pr view files field pagination implementation 2025 2026
💡 Result:
As of August 2026, the GitHub CLI (gh) has been updated to support pagination for the files and commits fields in the gh pr view --json command [1]. Historically, gh pr view --json files and gh pr view --json commits were limited to returning a maximum of 100 items, silently truncating any additional data without warning [2][3]. Because the underlying GraphQL fragments did not include pageInfo and lacked corresponding preloading functions in the CLI's codebase, users previously had to manually fetch complete data using the gh api graphql command with pagination parameters [4][3][5]. Following community feedback regarding this limitation [2][6], recent updates to the GitHub CLI have implemented native pagination for these fields [1]. This was achieved by: - Extending the pull request GraphQL fragments for files and commits to include pageInfo (hasNextPage and endCursor) [1]. - Adding PageInfo to the internal response structs [1]. - Implementing preloading functions (e.g., preloadPrFiles, preloadPrCommits) that automatically fetch subsequent pages beyond the initial 100-node limit, aligning the behavior with other paginated subresources like reviews and comments [1][2]. This change ensures that gh pr view --json returns the complete list of files and commits regardless of the total count [1].
Citations:
- 1: Paginate files and commits in
gh pr view --jsoncli/cli#13340 - 2:
gh pr view --json filesand--json commitssilently truncate at 100 items cli/cli#13338 - 3: The
pr viewcommand with the--json filesflag omits some of the files changed cli/cli#9916 - 4: Remove restrictions of
gh pr view --json filescli/cli#5368 - 5: Remove restrictions of
gh pr view --json filescli/cli#5368 - 6: `gh pr view --json files`: support paging, server-side filtering, or increased limits cli/cli#5373
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import urllib.request
urls = [
"https://raw.githubusercontent.com/cli/cli/trunk/pkg/cmd/pr/view/view.go",
"https://raw.githubusercontent.com/cli/cli/master/pkg/cmd/pr/view/view.go",
]
for url in urls:
try:
text = urllib.request.urlopen(
urllib.request.Request(url, headers={"User-Agent": "review-verifier"}),
timeout=15,
).read().decode()
print(f"--- {url} ---")
for i, line in enumerate(text.splitlines(), 1):
if "files" in line.lower() or "pageInfo" in line or "after" in line:
print(f"{i}:{line}")
break
except Exception as exc:
print(f"{url}: {exc}")
PYRepository: openshift/hypershift
Length of output: 575
Require pagination support for the gh CLI.
Older gh versions return only the first 100 entries for gh pr view --json files. Because this script does not specify a minimum version, users can miss workflow files. Use gh api --paginate "repos/$REPO/pulls/$pr_number/files" --jq '.[].filename', or enforce a gh version that supports pagination.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contrib/ci/gha-rehearse.sh` around lines 43 - 49, Update the workflow-file
discovery in the rehearse script to retrieve all pull-request files, replacing
the non-paginated gh pr view files query with gh api --paginate against the pull
request files endpoint and extracting each filename. Preserve the existing
workflow YAML filtering and workflow_files population, and continue deriving
head_branch from the pull-request metadata.
| local -a new_workflows=() | ||
| for wf in "${workflow_files[@]}"; do | ||
| if ! git cat-file -e "origin/${DEFAULT_BRANCH}:${wf}" 2>/dev/null; then | ||
| new_workflows+=("$wf") | ||
| fi | ||
| done |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
Fetch the default branch before the existence check; the stale check can clobber a real workflow.
Line 57 tests origin/${DEFAULT_BRANCH}:${wf} against the local remote-tracking ref. The script fetches that ref only later, at Line 89. If the local origin/${DEFAULT_BRANCH} is stale, the check reads an old tree.
The failure mode is destructive. Assume a workflow file was merged upstream after the last local fetch. The stale check classifies it as new. The script then generates a stub for it. Line 92 reads the index from the freshly fetched tip, which does contain the real file. Line 97 overwrites that entry with the stub blob. Line 110 pushes as a fast-forward, so the push succeeds and the real workflow content is replaced on the default branch.
Move the fetch above the existence loop.
🐛 Proposed fix
+ git fetch origin "${DEFAULT_BRANCH}" --quiet
+
local -a new_workflows=()
for wf in "${workflow_files[@]}"; do
if ! git cat-file -e "origin/${DEFAULT_BRANCH}:${wf}" 2>/dev/null; then
new_workflows+=("$wf")
fi
doneThen remove the now-redundant fetch at Line 89:
echo "Pushing stubs to ${DEFAULT_BRANCH}..."
- git fetch origin "${DEFAULT_BRANCH}" --quiet🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contrib/ci/gha-rehearse.sh` around lines 55 - 60, Move the fetch of
origin/${DEFAULT_BRANCH} in the rehearse script to before the new_workflows
existence-check loop, ensuring git cat-file evaluates the current
remote-tracking tree. Remove the later redundant fetch while preserving the
existing workflow generation and push flow.
| local -r tmpdir="$(mktemp -d)" | ||
| trap 'rm -rf "$tmpdir"' EXIT |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
The EXIT trap cannot see tmpdir, so the temporary directory leaks.
trap receives a single-quoted string. Bash expands $tmpdir when the trap fires, not when it is set. tmpdir is a local variable of cmd_setup. When cmd_setup returns normally and the script reaches the end at Line 148, tmpdir is out of scope. Under set -u the trap then fails with an unbound variable error, and the directory stays on disk.
Expand the path at trap-definition time.
🛡️ Proposed fix
local -r tmpdir="$(mktemp -d)"
- trap 'rm -rf "$tmpdir"' EXIT
+ # shellcheck disable=SC2064 # expand tmpdir now, not when the trap fires
+ trap "rm -rf -- '${tmpdir}'" EXIT📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local -r tmpdir="$(mktemp -d)" | |
| trap 'rm -rf "$tmpdir"' EXIT | |
| local -r tmpdir="$(mktemp -d)" | |
| # shellcheck disable=SC2064 # expand tmpdir now, not when the trap fires | |
| trap "rm -rf -- '${tmpdir}'" EXIT |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contrib/ci/gha-rehearse.sh` around lines 72 - 73, Update the EXIT trap in
cmd_setup to expand tmpdir when the trap is defined rather than when it
executes, while preserving safe cleanup of the temporary directory under set -u.
| for wf in "${new_workflows[@]}"; do | ||
| local name | ||
| name="$(git show "FETCH_HEAD:${wf}" 2>/dev/null \ | ||
| | grep -m1 '^name:' | sed 's/^name:\s*//' || echo "${wf##*/}")" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
\s in sed is a GNU extension and fails on BSD/macOS sed. Both name-extraction sites use sed 's/^name:\s*//'. GNU sed supports \s. BSD sed, which ships on macOS, does not. On macOS the pattern matches a literal s character instead of whitespace, so name: Unit Tests becomes Unit Tests with a leading space. The stub YAML and the printed gh workflow run command then carry the wrong name. The shebang is /usr/bin/env bash, so the script is not restricted to GNU userland. Replace \s with the POSIX class [[:space:]] at both sites.
contrib/ci/gha-rehearse.sh#L80-L80: changesed 's/^name:\s*//'tosed 's/^name:[[:space:]]*//'.contrib/ci/gha-rehearse.sh#L118-L118: changesed 's/^name:\s*//'tosed 's/^name:[[:space:]]*//'.
📍 Affects 1 file
contrib/ci/gha-rehearse.sh#L80-L80(this comment)contrib/ci/gha-rehearse.sh#L118-L118
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contrib/ci/gha-rehearse.sh` at line 80, In both name-extraction sites using
sed in contrib/ci/gha-rehearse.sh#L80-L80 and
contrib/ci/gha-rehearse.sh#L118-L118, replace the GNU-specific \s pattern with
the POSIX [[:space:]] class so whitespace is stripped correctly on BSD/macOS
sed.
| local -r parent="$(git rev-parse "origin/${DEFAULT_BRANCH}~1")" | ||
| git push origin "${parent}:refs/heads/${DEFAULT_BRANCH}" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
The cleanup push is a rewind and will be rejected without --force-with-lease.
Line 139 pushes the parent commit to the default branch. That moves the branch backwards, so it is not a fast-forward. Git rejects a non-fast-forward update on a plain git push. The documented cleanup path therefore fails, and the stub commit stays on the default branch.
Use --force-with-lease pinned to the tip that Line 126 fetched. The lease keeps the operation safe if another commit lands between the fetch and the push.
🐛 Proposed fix
echo "Reverting stub commit on ${DEFAULT_BRANCH}..."
+ local -r stub_tip="$(git rev-parse "origin/${DEFAULT_BRANCH}")"
local -r parent="$(git rev-parse "origin/${DEFAULT_BRANCH}~1")"
- git push origin "${parent}:refs/heads/${DEFAULT_BRANCH}"
+ git push --force-with-lease="refs/heads/${DEFAULT_BRANCH}:${stub_tip}" \
+ origin "${parent}:refs/heads/${DEFAULT_BRANCH}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local -r parent="$(git rev-parse "origin/${DEFAULT_BRANCH}~1")" | |
| git push origin "${parent}:refs/heads/${DEFAULT_BRANCH}" | |
| local -r stub_tip="$(git rev-parse "origin/${DEFAULT_BRANCH}")" | |
| local -r parent="$(git rev-parse "origin/${DEFAULT_BRANCH}~1")" | |
| git push --force-with-lease="refs/heads/${DEFAULT_BRANCH}:${stub_tip}" \ | |
| origin "${parent}:refs/heads/${DEFAULT_BRANCH}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@contrib/ci/gha-rehearse.sh` around lines 138 - 139, Update the cleanup push
in the rehearsal script to use --force-with-lease, pinning the lease to the
default branch tip fetched at line 126, while still pushing parent to
refs/heads/${DEFAULT_BRANCH}.
2 participants
What this PR does / why we need it:
Adds
contrib/ci/gha-rehearse.shto enable rehearsing new GitHub Actions workflows from PR branches before merging.GitHub Actions
workflow_dispatchrequires the workflow file to exist on the default branch for it to appear in the Actions UI. This script pushes minimal stubs to the default branch so an admin can select the PR branch and run the real workflow code, then cleans up the stub commit afterwards.setup <PR>: identifies new workflow files in a PR, pushesworkflow_dispatchstubs to the default branch, printsgh workflow runcommandscleanup: safely reverts the stub commit (refuses if HEAD is not a stub commit)Which issue(s) this PR fixes:
Fixes CNTRLPLANE-4013
Special notes for your reviewer:
Requires admin push access to the default branch. The script uses low-level git plumbing (read-tree, hash-object, write-tree, commit-tree) to build the stub commit without touching the working directory.
Checklist:
Summary by CodeRabbit