Add trigger-qe-tests Claude Code skill - #4322
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: rrasouli 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 |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds a Claude command for triggering WMCO Windows Container QE Z-Stream tests. It validates and normalizes a release version, selects winc periodic jobs, creates a release repository worktree and branch, renames job aliases, regenerates manifests, and opens a temporary non-draft PR. After the rehearsal notifier comment, it posts 🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
bc67b47 to
564a9cc
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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 @.claude/commands/trigger-qe-tests.md:
- Around line 48-53: Update both fenced code blocks in the command documentation
to declare the bash language, including the block showing the config path and
the block containing the grep command; leave their contents unchanged.
- Line 4: Constrain the tool permissions declared by the trigger-qe-tests
command, especially unrestricted Bash and Edit access, to the narrowest
supported scopes for its required operations. Update the workflow instructions
to require explicit user confirmation immediately before every external
mutation, including repository changes, pushes, pull requests, and Prow
commands.
- Around line 93-101: Update Step 5 to poll the specific PR for a
REHEARSALNOTIFIER comment at a defined interval until a bounded deadline; if the
deadline expires, fail clearly instead of continuing. Validate both the author
and presence of the expected rehearsal job list before posting /pj-rehearse, and
only report triggered jobs after a successful post.
- Around line 67-72: Update the release-repository workflow in the worktree
setup and related commands to verify the repository root and remote URLs before
any fetch, edit, or push; require the source checkout to be clean, create the
parent worktree directory, and abort when the target branch or worktree already
exists. Preserve existing resources unless explicit, verified confirmation
authorizes reuse or deletion, including the related sections at the referenced
command steps.
- Around line 73-78: The workflow around the worktree job renaming and `make
update` must make manifest regeneration a mandatory gate: require `make update`
to exit successfully, verify the resulting diff contains the selected `winc-fNN`
to `winc-zstream-fNN` renames, and abort before commit or PR creation if either
check fails. Apply the same validation to the related workflow section
referenced by the comment.
- Around line 19-33: Update the version-handling instructions to validate the
input before using it in paths, branch names, or commands: accept only supported
numeric OCP/WMCO forms, reject malformed or unsupported values, and derive
RELEASE and WMCO_VERSION from parsed components rather than string
interpolation. Require shell-quoting for every version expansion used in
generated commands, while preserving the existing OCP/WMCO mapping and prompt
behavior for missing input.
🪄 Autofix (Beta)
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: 22958147-7e7f-4223-89aa-8a602b058953
📒 Files selected for processing (1)
.claude/commands/trigger-qe-tests.md
| --- | ||
| description: Create a DEBUG PR to trigger WMCO QE z-stream tests via /pj-rehearse | ||
| args: "[version]" | ||
| allowed-tools: Read, Edit, Bash, Grep, Glob, AskUserQuestion |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Constrain the command’s mutation authority.
Bash and Edit are granted without scope restrictions, while this command can fetch repositories, modify files, push branches, create PRs, and post Prow commands. For this high-risk .claude workflow, narrow tool scopes where supported and require explicit confirmation immediately before each external mutation.
🤖 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 @.claude/commands/trigger-qe-tests.md at line 4, Constrain the tool
permissions declared by the trigger-qe-tests command, especially unrestricted
Bash and Edit access, to the narrowest supported scopes for its required
operations. Update the workflow instructions to require explicit user
confirmation immediately before every external mutation, including repository
changes, pushes, pull requests, and Prow commands.
Source: Path instructions
| The version argument is required and must be an explicit release number (e.g., `5.0`, `4.21`, `4.20`, `10.21`, `11.0`). | ||
| If no version is provided, ask the user for one. Do not guess or resolve from branch names. | ||
|
|
||
| The user may pass either the OCP version or the WMCO version. The mapping is: | ||
| - WMCO `10.xx` = OCP `4.xx` (e.g., WMCO 10.20 = OCP 4.20) | ||
| - WMCO `11.x` = OCP `5.x` (e.g., WMCO 11.0 = OCP 5.0) | ||
|
|
||
| Normalize input to derive both versions: | ||
| - If user passes `10.xx` -> RELEASE=`4.xx`, WMCO_VERSION=`10.xx` | ||
| - If user passes `11.x` -> RELEASE=`5.x`, WMCO_VERSION=`11.x` | ||
| - If user passes `4.xx` -> RELEASE=`4.xx`, WMCO_VERSION=`10.xx` | ||
| - If user passes `5.x` -> RELEASE=`5.x`, WMCO_VERSION=`11.x` | ||
|
|
||
| Use `RELEASE` (OCP version) for config file paths and branch names. | ||
| Use `WMCO_VERSION` in the PR title and body. |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Validate version before interpolating it into paths and commands.
The instructions require an “explicit release number” but define no validation step. Reject anything outside the supported numeric forms, derive the normalized versions from parsed components, and shell-quote every expansion; otherwise malformed input can select unintended files/worktrees or become command syntax when substituted into generated shell commands.
🤖 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 @.claude/commands/trigger-qe-tests.md around lines 19 - 33, Update the
version-handling instructions to validate the input before using it in paths,
branch names, or commands: accept only supported numeric OCP/WMCO forms, reject
malformed or unsupported values, and derive RELEASE and WMCO_VERSION from parsed
components rather than string interpolation. Require shell-quoting for every
version expansion used in generated commands, while preserving the existing
OCP/WMCO mapping and prompt behavior for missing input.
Source: Path instructions
| ``` | ||
| ~/Documents/GitHub/release/ci-operator/config/openshift/openshift-tests-private/openshift-openshift-tests-private-release-{RELEASE}__amd64-nightly.yaml | ||
| ``` | ||
| 2. Confirm it exists and contains `winc` jobs: | ||
| ```bash | ||
| grep 'as:.*winc.*f[0-9]' <config-file> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add languages to both fenced code blocks.
This is reported by markdownlint MD040. Mark both fences as bash.
Proposed fix
- ```
+ ```bash
...
- ```
+ ```bashAlso applies to: 97-100
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 48-48: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 @.claude/commands/trigger-qe-tests.md around lines 48 - 53, Update both
fenced code blocks in the command documentation to declare the bash language,
including the block showing the config path and the block containing the grep
command; leave their contents unchanged.
Source: Linters/SAST tools
| 1. In the release repo, fetch upstream and create a worktree: | ||
| ```bash | ||
| cd ~/Documents/GitHub/release | ||
| git fetch upstream | ||
| git worktree add ../worktrees/winc-zstream-{RELEASE} -b winc-zstream-{RELEASE} upstream/main | ||
| ``` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Verify repository identity and state before fetching, editing, or pushing.
These commands assume the fixed path, upstream, origin, branch, and worktree are all correct. Check the repository root and remote URLs, require a clean source checkout, create the parent worktree directory, and abort if the branch or worktree already exists. Never interpret “ask before overwriting” as permission to delete or reuse an existing branch without an explicit, verified confirmation.
Also applies to: 83-90, 110-110
🤖 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 @.claude/commands/trigger-qe-tests.md around lines 67 - 72, Update the
release-repository workflow in the worktree setup and related commands to verify
the repository root and remote URLs before any fetch, edit, or push; require the
source checkout to be clean, create the parent worktree directory, and abort
when the target branch or worktree already exists. Preserve existing resources
unless explicit, verified confirmation authorizes reuse or deletion, including
the related sections at the referenced command steps.
Source: Path instructions
| 2. In the worktree config file, for each selected job, rename the `as:` field: | ||
| - Pattern: `winc-fNN` -> `winc-zstream-fNN` | ||
| - Example: `aws-ipi-ovn-winc-f14` -> `aws-ipi-ovn-winc-zstream-f14` | ||
| - Only rename jobs that do NOT already contain `zstream` | ||
| 3. Run `make update` to regenerate the Prow job files (ask user to run this). | ||
| 4. Show the diff for review. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make manifest regeneration a mandatory success gate.
make update produces the generated ci-operator/jobs/ files required by the PR, but the workflow only asks the user to run it and does not require a successful exit or verify that the expected generated file changed. Abort before commit/PR creation unless regeneration succeeds and the resulting diff contains the selected job renames.
Also applies to: 82-90
🤖 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 @.claude/commands/trigger-qe-tests.md around lines 73 - 78, The workflow
around the worktree job renaming and `make update` must make manifest
regeneration a mandatory gate: require `make update` to exit successfully,
verify the resulting diff contains the selected `winc-fNN` to `winc-zstream-fNN`
renames, and abort before commit or PR creation if either check fails. Apply the
same validation to the related workflow section referenced by the comment.
| ### Step 5: Trigger rehearsals | ||
|
|
||
| 1. Wait for the REHEARSALNOTIFIER bot comment (poll PR comments). | ||
| 2. Once the rehearsal list appears, post: | ||
| ``` | ||
| /pj-rehearse | ||
| ``` | ||
| This triggers up to 5 rehearsals. If more than 5 jobs, use `/pj-rehearse max`. | ||
| 3. Report the PR URL and triggered job names. |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Bound notifier polling and fail closed.
“Wait” has no timeout, polling interval, or failure path, so the command can hang indefinitely. Poll the specific PR with a bounded deadline, verify the comment is from REHEARSALNOTIFIER, and only post /pj-rehearse once the expected job list is present.
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 97-97: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 @.claude/commands/trigger-qe-tests.md around lines 93 - 101, Update Step 5 to
poll the specific PR for a REHEARSALNOTIFIER comment at a defined interval until
a bounded deadline; if the deadline expires, fail clearly instead of continuing.
Validate both the author and presence of the expected rehearsal job list before
posting /pj-rehearse, and only report triggered jobs after a successful post.
0f6fa13 to
a7b48e6
Compare
jrvaldes
left a comment
jrvaldes
left a comment
There was a problem hiding this comment.
@rrasouli thanks for proposing this.
I'd prefer to see a bash script under e.g. hack/trigger-qe-tests.sh that:
Uses the GitHub CLI (gh) for the PR/comment operations instead of hand-rolled shell snippets in a command manifest.
Lets the caller pass the path to their local openshift/release checkout as an argument or env var, falling back to cloning in a temp dir.
Validates the version, checks repo/remotes, renames the jobs, runs make update, creates the PR, and posts /pj-rehearse in one reproducible flow.
It's also much cheaper for the team, since it doesn't burn tokens on AI usage every time someone needs to trigger the QE jobs.
|
|
||
| Show the diff and ask the user for explicit confirmation before committing, pushing, or creating the PR. | ||
| ```bash | ||
| cd ~/Documents/GitHub/worktrees/winc-zstream-{RELEASE} |
There was a problem hiding this comment.
assumes the openshift/release repo always lives at ~/Documents/GitHub/release. That path is specific to one person's laptop setup and won't work for anyone who keeps their repos elsewhere (e.g. ~/git/, ~/src/, /var/home/..., or a non-macOS layout).
a7b48e6 to
f638d76
Compare
f638d76 to
76d05d9
Compare
|
@rrasouli: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
/close in favor #4324 we will run hack script instead of Claude skill |
1 similar comment
|
/close in favor #4324 we will run hack script instead of Claude skill |
2 participants
Adds a Claude Code slash command
/trigger-qe-tests [version]that automates creating DEBUG PRs in openshift/release to trigger WMCO QE z-stream tests via /pj-rehearse.The skill:
Reference: openshift/release#81646
Summary by CodeRabbit