NO-ISSUE: Release manager helper using Claude skills#6351
NO-ISSUE: Release manager helper using Claude skills#6351ggiguash wants to merge 10 commits intoopenshift:mainfrom
Conversation
|
@ggiguash: This pull request explicitly references no jira issue. 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. |
|
Skipping CI for Draft Pull Request. |
WalkthroughAdds three CLI specification documents for CI analysis, two Bash scripts to query MicroShift Prow jobs (release and PR-focused), and broadens .claude runtime permissions; generated reports are written under /tmp (text summaries and a self-contained HTML report). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 Tip You can validate your CodeRabbit configuration file in your editor.If your editor has YAML language server, you can enable auto-completion and validation by adding |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ggiguash The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@coderabbitai perform full review |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
.claude/commands/analyze-ci-for-release-manager.md (1)
11-13: Add language identifiers to fenced code blocks.Static analysis flags missing language specifiers. Use
textorbashas appropriate.Also applies to: 38-40, 46-48, 190-198, 203-205, 208-210, 213-215
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/analyze-ci-for-release-manager.md around lines 11 - 13, Several fenced code blocks in .claude/commands/analyze-ci-for-release-manager.md are missing language identifiers; update each triple-backtick block (e.g. the block containing `/analyze-ci-for-release-manager <release1,release2,...>`) to use an appropriate language tag such as `text` or `bash`, and apply the same change to the other reported blocks (lines around 38-40, 46-48, 190-198, 203-205, 208-210, 213-215) so all fenced code blocks include a language specifier..claude/commands/analyze-ci-for-release.md (1)
14-16: Add language identifiers to fenced code blocks.Static analysis flags these. Use
text,bash, or appropriate language.Also applies to: 50-52, 76-81, 131-182, 188-190, 199-201, 210-212, 236-239, 242-246, 249-252, 264-267, 270-273, 276-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/analyze-ci-for-release.md around lines 14 - 16, Several fenced code blocks (for example the one showing "/analyze-ci-for-release <release-version> [--limit N]") are missing language identifiers; update every fenced block in this document to include an appropriate language tag (e.g., text, bash, json) so Markdown linters stop flagging them—search for the visible fenced snippets such as command usages, shell examples, and multi-line outputs (including the other blocks noted in the review) and add the correct language identifier after the opening ``` for each..claude/scripts/microshift-prow-jobs-for-release.sh (1)
30-61: Status-to-icon logic is duplicated.The same mapping exists in
status_to_icon()(unused) and inline in both jq queries. Consider consolidating.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/scripts/microshift-prow-jobs-for-release.sh around lines 30 - 61, Extract the duplicated status-to-icon logic into a single reusable jq function (e.g. def status_to_icon: if . == "success" then "✓" elif . == "failure" then "✗" elif . == "pending" then "⋯" else "?" end) and include that function at the top of both jq program strings used by query_status() and query_failed(), replacing the inline mapping expressions; also remove the unused shell-level status_to_icon() helper (or make it call through to the jq function if you prefer a shell wrapper) so the mapping is defined once and reused by both query_status() and query_failed().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/analyze-ci-for-release.md:
- Line 4: The description string in analyze-ci-for-release.md uses the wrong
skill name "analyse-ci-test-job"; update that description to use the exact skill
identifier "analyze-ci-test-job" so it matches the entry in
.claude/settings.json and avoids invocation failures—locate the description line
in analyze-ci-for-release.md and replace "analyse-ci-test-job" with
"analyze-ci-test-job".
In @.claude/scripts/microshift-prow-jobs-for-release.sh:
- Around line 13-16: The curl call that fetches "${PROW_URL}" can hang
indefinitely; update the curl invocation (the line using curl -s "${PROW_URL}"
before piping to jq) to include a timeout option (e.g. --max-time or
--connect-timeout and --max-time) so the script fails fast when the Prow server
is slow or unresponsive and still pipes output to jq for the existing filter
using ${release} and ${PROW_JOB}.
- Around line 93-95: The branch handling the --mode option assigns mode="${2}"
and then does shift 2 which will fail under set -u if no value is provided;
update the case arm for --mode to validate the next positional exists and is not
another flag before consuming it (for example check that "${2:-}" is non-empty
and does not start with '-' or ensure $# -ge 2), and if the check fails call the
usage/error path; otherwise set mode="$2" and shift 2 as before. Include
references to the --mode case, the mode variable, and the shift 2 operation so
reviewers can find and apply the guard.
In @.claude/settings.json:
- Around line 5-6: The setting "Bash(*)" in .claude/settings.json grants
unrestricted shell execution; restrict it by replacing "Bash(*)" with explicit,
minimal patterns for only the commands the tools need (for example
"Bash(curl:*)", "Bash(jq:*)", or similar command-specific patterns) and keep
"Write(//tmp/**)" if needed; update the Bash entry(s) to the smallest possible
whitelist to limit scope and remove any broad wildcard usage.
---
Nitpick comments:
In @.claude/commands/analyze-ci-for-release-manager.md:
- Around line 11-13: Several fenced code blocks in
.claude/commands/analyze-ci-for-release-manager.md are missing language
identifiers; update each triple-backtick block (e.g. the block containing
`/analyze-ci-for-release-manager <release1,release2,...>`) to use an appropriate
language tag such as `text` or `bash`, and apply the same change to the other
reported blocks (lines around 38-40, 46-48, 190-198, 203-205, 208-210, 213-215)
so all fenced code blocks include a language specifier.
In @.claude/commands/analyze-ci-for-release.md:
- Around line 14-16: Several fenced code blocks (for example the one showing
"/analyze-ci-for-release <release-version> [--limit N]") are missing language
identifiers; update every fenced block in this document to include an
appropriate language tag (e.g., text, bash, json) so Markdown linters stop
flagging them—search for the visible fenced snippets such as command usages,
shell examples, and multi-line outputs (including the other blocks noted in the
review) and add the correct language identifier after the opening ``` for each.
In @.claude/scripts/microshift-prow-jobs-for-release.sh:
- Around line 30-61: Extract the duplicated status-to-icon logic into a single
reusable jq function (e.g. def status_to_icon: if . == "success" then "✓" elif .
== "failure" then "✗" elif . == "pending" then "⋯" else "?" end) and include
that function at the top of both jq program strings used by query_status() and
query_failed(), replacing the inline mapping expressions; also remove the unused
shell-level status_to_icon() helper (or make it call through to the jq function
if you prefer a shell wrapper) so the mapping is defined once and reused by both
query_status() and query_failed().
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 96f88164-5cb9-45af-821b-c07181a022a7
📒 Files selected for processing (4)
.claude/commands/analyze-ci-for-release-manager.md.claude/commands/analyze-ci-for-release.md.claude/scripts/microshift-prow-jobs-for-release.sh.claude/settings.json
|
/test verify |
|
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/analyze-ci-for-release-manager.md:
- Around line 11-13: The markdown contains unlabeled fenced code blocks (for
example the fence with "/analyze-ci-for-release-manager
<release1,release2,...>") which triggers MD040; update each unlabeled
triple-backtick fence (including the other ranges mentioned) by adding an
appropriate language identifier such as bash, text, or html so the fences become
like ```bash or ```text; ensure you pick the label that best matches the snippet
content for consistent rendering and markdownlint compliance.
- Around line 163-164: The tab switching relies on an implicit global event;
update the inline handlers to pass the event explicitly (e.g., call
showTab(event, 'periodics') / showTab(event, 'pull-requests') instead of
showTab('...')) and change the showTab function signature to accept an event
parameter (e.g., function showTab(e, tabName)) and use e.target instead of
event.target; also update any other calls/usages of showTab (the other
occurrences around the tab-handling block) to the new two-argument form so they
no longer rely on the implicit global event.
In @.claude/commands/analyze-ci-for-release.md:
- Around line 22-23: Replace all occurrences of the mixed agent identifiers so
the document uses a single callable name "analyze-ci-test-job" everywhere
instead of "openshift-ci-test-job"; update every reference (e.g., the lines
showing "openshift-ci-test-job" and "analyze-ci-test-job" at the noted sections
and any headings, examples, or step lists) to the canonical
"analyze-ci-test-job" to ensure deterministic orchestration and consistent
invocation.
- Around line 14-16: Add explicit language identifiers to all fenced code blocks
flagged by markdownlint (MD040) so the blocks render and lint correctly; e.g.,
mark shell/command examples like "/analyze-ci-for-release <release-version>
[--limit N]" with a bash (or sh) tag and use text/plain or none for non-code
output blocks — update every fenced block mentioned in the review (including the
command example and the other flagged blocks) to include the appropriate
language tag.
In @.claude/settings.json:
- Around line 8-12: The entries Skill(analyze-ci-for-release-manager),
Skill(analyze-ci-for-release), Skill(analyze-ci-test-job),
Skill(analyze-ci-test-scenario), and Skill(analyze-sos-report) in the settings
are referencing command files (they exist under .claude/commands/) but not
actual skill/agent definitions, which will fail at runtime; fix by either (A)
adding matching agent/skill definitions named exactly
analyze-ci-for-release-manager, analyze-ci-for-release, analyze-ci-test-job,
analyze-ci-test-scenario, and analyze-sos-report under your agents/skills store
so Skill(...) points to real agents, or (B) remove these Skill(...) grants and
replace them with the appropriate slash-command invocations (or Command(...)
entries) that reference the existing command definitions (e.g., use the command
identifier for analyze-ci-for-release-manager etc.), ensuring the settings.json
entries match the actual artifact types present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 70c7b89e-9758-4d69-b1c8-128c64cddd1b
📒 Files selected for processing (6)
.claude/commands/analyze-ci-for-pull-requests.md.claude/commands/analyze-ci-for-release-manager.md.claude/commands/analyze-ci-for-release.md.claude/scripts/microshift-prow-jobs-for-pull-requests.sh.claude/scripts/microshift-prow-jobs-for-release.sh.claude/settings.json
|
/verified by @ggiguash |
|
@ggiguash: This PR has been marked as verified by 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. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
.claude/commands/analyze-ci-for-pull-requests.md (1)
37-37: Include aborted jobs in the analysis set, not only✗failures.Line 37 currently scopes parsing to
✗, but the producer script also emits⊘(aborted). Treating both as non-success avoids missing actionable CI breakages.Suggested doc patch
-2. Parse the output to identify PRs with failed jobs (lines containing `✗`) +2. Parse the output to identify PRs with non-success jobs (lines containing `✗` or `⊘`)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/commands/analyze-ci-for-pull-requests.md at line 37, The parsing step currently only looks for lines containing the failure marker `✗`; update the parser described in "Parse the output to identify PRs with failed jobs (lines containing `✗`)" to also treat the aborted marker `⊘` as a non-success state. Change the logic/regex that matches `✗` so it matches either `✗` or `⊘` (e.g., use an alternation or character class) and ensure any downstream filtering/aggregation that flags PRs as failing includes both markers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/commands/analyze-ci-for-pull-requests.md:
- Around line 11-13: The markdown has unlabeled fenced code blocks triggering
MD040; update all fences in the analyze-ci-for-pull-requests documentation so
command examples like "/analyze-ci-for-pull-requests [--rebase] [--limit N]" and
the standalone "/analyze-ci-for-pull-requests" use ```bash, and output/result
blocks such as the PR sample (starting "=== PR `#6313`...") and "No open pull
requests found." use ```text; ensure every remaining unlabeled fence (including
the blocks around the PR sample and other listed ranges) is given the proper
language identifier to clear the markdownlint warning.
In @.claude/commands/analyze-ci-for-release-manager.md:
- Around line 244-247: Update the documentation steps that say "Convert the
plain text analysis reports into HTML-formatted content, preserving all
information" and "Ensure all Prow job URLs from the original analyses remain
clickable links in the HTML" to require HTML-escaping of the analysis text
before any HTML injection, then perform URL linkification on the escaped text
(so links remain clickable but content cannot execute scripts); explicitly state
the ordering (escape first, then linkify) and add a note to preserve badge color
handling unchanged while using the escaped-and-linkified output for final HTML
assembly.
---
Nitpick comments:
In @.claude/commands/analyze-ci-for-pull-requests.md:
- Line 37: The parsing step currently only looks for lines containing the
failure marker `✗`; update the parser described in "Parse the output to identify
PRs with failed jobs (lines containing `✗`)" to also treat the aborted marker
`⊘` as a non-success state. Change the logic/regex that matches `✗` so it
matches either `✗` or `⊘` (e.g., use an alternation or character class) and
ensure any downstream filtering/aggregation that flags PRs as failing includes
both markers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: c0b3dfff-47bc-4f52-88c1-1929a522ff6e
📒 Files selected for processing (5)
.claude/agents/openshift-ci-analysis.md.claude/commands/analyze-ci-for-pull-requests.md.claude/commands/analyze-ci-for-release-manager.md.claude/commands/analyze-ci-for-release.md.claude/settings.json
| ``` | ||
| /analyze-ci-for-pull-requests [--rebase] [--limit N] | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to remaining fenced code blocks (MD040).
Several fences are still unlabeled, which keeps markdownlint warnings active. Use bash for command snippets and text for output blocks.
Suggested doc patch
-```
+```bash
/analyze-ci-for-pull-requests [--rebase] [--limit N]- +text
=== PR #6313: USHIFT-6636: Change test-agent impl to align with greenboot-rs ===
#6313
...
-```
+```bash
/analyze-ci-for-pull-requests
- +text
No open pull requests found.
Also applies to: 55-63, 126-174, 180-182, 192-194, 202-204, 229-231, 234-237, 240-243
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 11-11: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/analyze-ci-for-pull-requests.md around lines 11 - 13, The
markdown has unlabeled fenced code blocks triggering MD040; update all fences in
the analyze-ci-for-pull-requests documentation so command examples like
"/analyze-ci-for-pull-requests [--rebase] [--limit N]" and the standalone
"/analyze-ci-for-pull-requests" use ```bash, and output/result blocks such as
the PR sample (starting "=== PR `#6313`...") and "No open pull requests found."
use ```text; ensure every remaining unlabeled fence (including the blocks around
the PR sample and other listed ranges) is given the proper language identifier
to clear the markdownlint warning.
| - Do NOT re-analyze or reinterpret the data from `analyze-ci-for-release` or `analyze-ci-for-pull-requests` - use their output as-is | ||
| - Convert the plain text analysis reports into HTML-formatted content, preserving all information | ||
| - Ensure all Prow job URLs from the original analyses remain clickable links in the HTML | ||
| - Use appropriate badge colors: |
There was a problem hiding this comment.
Escape report content before HTML injection.
Line 245/246 guidance can lead to raw analysis text being embedded directly into HTML. Require HTML-escaping first, then linkify URLs, so CI/log content cannot execute scripts in the generated report.
Suggested doc patch
-- Convert the plain text analysis reports into HTML-formatted content, preserving all information
-- Ensure all Prow job URLs from the original analyses remain clickable links in the HTML
+- Convert plain text analysis reports into HTML safely: **escape all content first** (`& < > " '`)
+- After escaping, linkify only trusted URL patterns (e.g., `https://prow.ci.openshift.org/...`) so links stay clickable without allowing raw HTML/JS injection🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/commands/analyze-ci-for-release-manager.md around lines 244 - 247,
Update the documentation steps that say "Convert the plain text analysis reports
into HTML-formatted content, preserving all information" and "Ensure all Prow
job URLs from the original analyses remain clickable links in the HTML" to
require HTML-escaping of the analysis text before any HTML injection, then
perform URL linkification on the escaped text (so links remain clickable but
content cannot execute scripts); explicitly state the ordering (escape first,
then linkify) and add a note to preserve badge color handling unchanged while
using the escaped-and-linkified output for final HTML assembly.
|
@ggiguash: 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. |
Overview
microshift-prow-jobs-for-release.shto extract the last failed jobs per given release to be analyzedmicroshift-prow-jobs-for-pull-request.shto extract the list of failed jobs for open pull requests, filtered by rebase PRs onlyanalyze-ci-for-releaseandanalyze-ci-for-pull-requestsskills use the existingopenshift-ci-analysisskill to run on the failed job URLs and produce text summariesanalyze-ci-for-release-managerskill allows analysis of multiple releases usinganalyze-ci-for-releaseandanalyze-ci-for-pull-requests, producing an HTML summary.Example of command execution
Mar 16 2026 Status of the example command
HTML summary
List of failed 4.18 jobs
List of open rebase PRs
$ .claude/scripts/microshift-prow-jobs-for-pull-requests.sh --filter 'NO-ISSUE: rebase-release' Fetching open PRs... Fetching job results... PR TITLE ✓ ✗ ⋯ JOBS https://github.com/openshift/microshift/pull/6357 NO-ISSUE: rebase-release-4.21-4.21.0-0.nightly-... 7 0 6 13 https://github.com/openshift/microshift/pull/6356 NO-ISSUE: rebase-release-4.18-4.18.0-0.nightly-... 6 0 6 12 https://github.com/openshift/microshift/pull/6355 NO-ISSUE: rebase-release-4.19-4.19.0-0.nightly-... 7 0 6 13 https://github.com/openshift/microshift/pull/6354 NO-ISSUE: rebase-release-4.20-4.20.0-0.nightly-... 7 0 6 13 https://github.com/openshift/microshift/pull/6353 NO-ISSUE: rebase-release-4.17-4.17.0-0.nightly-... 6 0 13 19