Skip to content

Payload analysis skill improvements - #360

Merged
openshift-merge-bot[bot] merged 11 commits into
openshift-eng:mainfrom
stbenjam:improvements
Mar 6, 2026
Merged

openshift-merge-bot[bot] merged 11 commits into
openshift-eng:mainfrom
stbenjam:improvements

Conversation

@stbenjam

@stbenjam stbenjam commented Mar 5, 2026

Copy link
Copy Markdown
Member

Improvements to payload analysis and related skills:

  • Extract trigger-payload-job skill — consolidates payload trigger logic (command selection, polling, URL collection) previously duplicated in bisect-payload-suspects and stage-payload-reverts
  • Surface retry attempt URLs — fetch_payloads.py outputs previousAttemptURLs; analyze-payload passes them to subagents to compare failure modes across retries
  • Skip previous attempts for aggregated jobs — aggregated job retries only re-run the analysis, not the underlying jobs, so only the most recent attempt is examined
  • Remove hardcoded tool references — subagent instructions no longer prescribe specific tool names
  • Fix trigger-payload-job contract — return no_response (not triggered) when bot never replies; fix YAML result format
  • Add no_retries enum valueretries_consistent now distinguishes jobs with zero retries from those where only the final attempt was examined
  • Fail-fast validation for aggregated jobs missing underlying_job_name
  • Job trigger throughput limits and skipped-job reporting

During a payload-agent run against 4.22.0-0.nightly-2026-03-05-125331,
two problems were discovered:

1. The agent used `/payload 4.22 nightly hypershift-ovn-conformance`
   instead of `/payload-aggregate <underlying-job-name> 10`. The payload
   trigger logic (aggregated vs non-aggregated commands, polling for bot
   response, URL extraction) was duplicated inline in both
   bisect-payload-suspects and stage-payload-reverts, making it easy to
   skip or get wrong. Extract this into a dedicated trigger-payload-job
   skill that both skills now reference as the single source of truth.

2. The agent only examined the final retry of each failed job, missing
   diagnostic signal from previous attempts. The release controller API
   now returns `previousAttemptURLs` for retried jobs. Update
   fetch_payloads.py to surface these URLs, and update analyze-payload
   to instruct subagents to compare failure modes across all retry
   attempts — consistent failures across retries strongly indicate a
   product regression vs intermittent/infrastructure issues.

Changes:
- New skill: trigger-payload-job — handles /payload-aggregate vs
  /payload-job command selection, idempotency, polling, URL collection
- bisect-payload-suspects: replace inline Steps 1.3-1.4 with reference
  to trigger-payload-job
- stage-payload-reverts: replace inline Substep 3 with reference to
  trigger-payload-job
- payload-agent: add note about populating is_aggregated and
  underlying_job_name for the trigger skill
- fetch_payloads.py: output previousAttemptURLs for retried jobs
- analyze-payload: pass all attempt URLs to subagents, add
  retries_consistent and retry_summary to structured return format

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@openshift-ci
openshift-ci Bot requested review from rvanderp3 and zaneb March 5, 2026 19:21
@openshift-ci openshift-ci Bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Mar 5, 2026
@coderabbitai

coderabbitai Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a new trigger-payload-job skill, delegates payload triggering to it across CI skills, propagates previousAttemptURLs and retry history through analysis, introduces global job-trigger limits and aggregated-job validations, updates fetch/formatting to list prior attempts, and bumps CI plugin version to 0.0.22.

Changes

Cohort / File(s) Summary
Version Bumps
\.claude-plugin/marketplace.json, plugins/ci/.claude-plugin/plugin.json, docs/data.json
Bumped CI plugin version from 0.0.210.0.22. docs/data.json also registers the new public skill trigger-payload-job.
New Skill: Trigger Payload Job
plugins/ci/skills/trigger-payload-job/SKILL.md
Adds a self-contained skill spec to post payload commands on PRs, perform idempotency checks, poll for bot replies, extract payload_test_url and per-job Prow URLs, enforce per-invocation limits, and return structured results (prow_jobs, skipped_jobs, etc.).
Payload Analysis & Retry History
plugins/ci/skills/analyze-payload/SKILL.md, plugins/ci/skills/fetch-payloads/SKILL.md, plugins/ci/skills/fetch-payloads/fetch_payloads.py
Propagates previousAttemptURLs and retry metadata through lookback and per-job failure-history logic, updates subagent invocation to include all attempt URLs, adds retries_consistent/retry_summary to results, and formats prior-attempt URLs in outputs.
Skill Integrations & Flow Changes
plugins/ci/skills/bisect-payload-suspects/SKILL.md, plugins/ci/skills/stage-payload-reverts/SKILL.md, plugins/ci/skills/payload-agent/SKILL.md
Replaces manual triggering steps with calls to trigger-payload-job; adds global triggering limits (caps for non-aggregated/aggregated jobs), prioritization/skip semantics, aggregated-job metadata requirements (is_aggregated, underlying_job_name), and fail-fast validation for aggregated jobs.
Formatting / Docs minor
docs/data.json (skill entry)
Added public skill entry trigger-payload-job with id trigger-payload-job, name and description.

Sequence Diagram

sequenceDiagram
    actor Agent as CI Agent
    participant PR as GitHub PR
    participant Bot as Payload Bot
    participant Prow as Prow System

    Agent->>Agent: Idempotency check (prior bot runs)
    Agent->>PR: Post `/payload-job` or `/payload-aggregate` comment(s)
    PR->>Bot: Notify bot of comment
    Bot->>Prow: Trigger validation jobs
    Bot->>PR: Reply with payload_test_url and prow job URLs (including previousAttemptURLs)
    Agent->>Bot: Poll for bot reply (finite retries)
    Agent->>Agent: Extract payload_test_url, prow_jobs, previousAttemptURLs, skipped_jobs
    Agent->>Agent: Return structured result (pr_url, comment_url, payload_test_url, prow_jobs, skipped_jobs, status, error)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 8
✅ Passed checks (8 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main changes: extracting a new trigger-payload-job skill and surfacing retry attempt URLs, which are the two core objectives of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
No Real People Names In Style References ✅ Passed All modified files comply with the requirement to avoid real people's names in plugin commands, skill documentation, example prompts, and style references.
No Assumed Git Remote Names ✅ Passed PR #360 modified files contain no hardcoded git remote names like 'origin' or 'upstream' in any git commands.
Git Push Safety Rules ✅ Passed PR does not introduce new git push operations; new trigger-payload-job skill avoids autonomous git operations, only posting PR comments and polling results.
No Untrusted Mcp Servers ✅ Passed No MCP server installations found. Repository contains only CI plugin updates with no untrusted external dependencies.
Ai-Helpers Overlap Detection ✅ Passed The PR introduces a new trigger-payload-job skill that consolidates previously duplicated payload-triggering logic from existing skills, representing intentional refactoring rather than overlapping functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
plugins/ci/skills/bisect-payload-suspects/SKILL.md (1)

125-130: ⚠️ Potential issue | 🟡 Minor

Example confidence score conflicts with the stated suspect range

Line 12 defines bisect suspects as medium-confidence (60-84), but Line 129 shows confidence_score: 42 in deferred_suspects. That inconsistency can cause incorrect caller behavior when constructing deferred entries.

Suggested doc fix
 deferred_suspects:
   - url: "https://github.com/openshift/mco/pull/5509"
     number: 5509
     component: "machine-config-operator"
-    confidence_score: 42
+    confidence_score: 61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/ci/skills/bisect-payload-suspects/SKILL.md` around lines 125 - 130,
The example in SKILL.md shows a deferred_suspects entry with confidence_score:
42 which conflicts with the documented medium-confidence suspect range (60-84);
update the example so the deferred_suspects confidence_score falls inside the
stated range (e.g., set confidence_score to a value between 60 and 84) or adjust
the documented suspect range to match the example, and ensure the
deferred_suspects and any related bisect suspects examples consistently
reference the same confidence range.
plugins/ci/skills/stage-payload-reverts/SKILL.md (1)

101-120: ⚠️ Potential issue | 🟠 Major

Return schema is missing per-job URL fields required by Substep 3

Line 101 requires recording individual prow_urls, but the STAGED_REVERT_RESULT schema has no field for per-job URLs (only payload_jobs_triggered). This creates an output contract mismatch and can drop diagnostic data.

Suggested schema update
 STAGED_REVERT_RESULT:
 - original_pr_url: ...
 - original_pr_number: ...
 - component: ...
 - jira_key: TRT-XXXX
 - jira_url: https://issues.redhat.com/browse/TRT-XXXX
 - revert_pr_url: https://github.com/org/repo/pull/YYYY
 - payload_test_url: https://pr-payload-tests.ci.openshift.org/runs/ci/...
-- payload_jobs_triggered: job1, job2, ...
+- payload_jobs_triggered:
+  - job_name: job1
+    prow_url: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/...
+  - job_name: job2
+    prow_url: https://prow.ci.openshift.org/view/gs/test-platform-results/logs/...
 - status: success|partial|failed
 - reused: none|jira|revert_pr|payload_jobs|all
 - error: none|description
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/ci/skills/stage-payload-reverts/SKILL.md` around lines 101 - 120, The
STAGED_REVERT_RESULT schema lacks per-job URL fields required by Step 3; add a
new field (e.g., payload_job_urls or payload_job_prow_urls) to the
STAGED_REVERT_RESULT return format that maps each entry from
payload_jobs_triggered to its prow_url (or is a list of objects with job and
prow_url), and update any code that builds/emits STAGED_REVERT_RESULT (the
subagent return generation) to populate payload_job_urls alongside
payload_test_url and payload_jobs_triggered so each triggered job's prow_url is
preserved for diagnostics.
🧹 Nitpick comments (1)
plugins/ci/skills/trigger-payload-job/SKILL.md (1)

33-41: Narrow idempotency reuse criteria to avoid stale run reuse.

Reusing any prior pr-payload-tests URL on the PR can hide fresh triggers for a new commit or different job set. Consider matching on current head SHA and requested jobs before reusing.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/ci/skills/trigger-payload-job/SKILL.md` around lines 33 - 41, Narrow
the idempotency check that reuses prior pr-payload-tests URLs: when querying PR
comments via the gh api command shown, parse and only reuse a found
pr-payload-tests.ci.openshift.org/runs/ci/<uuid> URL if that comment/body
includes the current PR head SHA and the same requested job set; update the
matching logic that currently selects any comment containing "pr-payload-tests"
to also verify head SHA and requested job names before skipping to Step 3 (so
you avoid reusing stale runs from prior commits or different job lists).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/ci/skills/fetch-payloads/fetch_payloads.py`:
- Around line 144-146: The loop assumes info.get("previousAttemptURLs", []) is
always a list; make it defensive by normalizing and validating the value before
iterating: fetch the raw value (e.g., prev_raw =
info.get("previousAttemptURLs")), if prev_raw is None set previous = [], if it's
not a list (use isinstance check) skip or coerce safely (e.g., wrap a single
string into [prev_raw] or set to []), then iterate over previous as currently
done; update the block around the previousAttemptURLs use in fetch_payloads.py
(the info variable and the for i, prev_url in enumerate(...) block) to avoid
TypeError when the field is null or non-list.

In `@plugins/ci/skills/payload-agent/SKILL.md`:
- Around line 102-103: Add a fail-fast validation when assembling failing_jobs
before calling trigger-payload-job: in the bisect-payload-suspects and
stage-payload-reverts code paths (where failing_jobs is constructed and passed
to trigger-payload-job), check each suspect entry and if suspect.is_aggregated
is true and suspect.underlying_job_name is empty/null, do not enqueue/trigger
for that suspect, mark the suspect as a hard error in the report (e.g., add a
failure record or set a status field) and continue; ensure the validation runs
after subagent analysis results are applied so is_aggregated and
underlying_job_name come from analyze-payload Step 5.

In `@plugins/ci/skills/trigger-payload-job/SKILL.md`:
- Around line 35-38: The gh api call that reads issue comments uses the default
ascending order and 30-per-page which can pick stale bot comments; modify the
request URL used in the command string "gh api
\"repos/<org>/<repo>/issues/<pr_number>/comments\"" to include explicit query
params (e.g. ?per_page=100&sort=created&direction=desc) and update the jq
expression to pick the first matching item (replace 'last' with '.[0]' or
similar) so you always get the most recent openshift-ci[bot] pr-payload-tests
comment; apply the same change to the other occurrence noted (lines 71-73).

---

Outside diff comments:
In `@plugins/ci/skills/bisect-payload-suspects/SKILL.md`:
- Around line 125-130: The example in SKILL.md shows a deferred_suspects entry
with confidence_score: 42 which conflicts with the documented medium-confidence
suspect range (60-84); update the example so the deferred_suspects
confidence_score falls inside the stated range (e.g., set confidence_score to a
value between 60 and 84) or adjust the documented suspect range to match the
example, and ensure the deferred_suspects and any related bisect suspects
examples consistently reference the same confidence range.

In `@plugins/ci/skills/stage-payload-reverts/SKILL.md`:
- Around line 101-120: The STAGED_REVERT_RESULT schema lacks per-job URL fields
required by Step 3; add a new field (e.g., payload_job_urls or
payload_job_prow_urls) to the STAGED_REVERT_RESULT return format that maps each
entry from payload_jobs_triggered to its prow_url (or is a list of objects with
job and prow_url), and update any code that builds/emits STAGED_REVERT_RESULT
(the subagent return generation) to populate payload_job_urls alongside
payload_test_url and payload_jobs_triggered so each triggered job's prow_url is
preserved for diagnostics.

---

Nitpick comments:
In `@plugins/ci/skills/trigger-payload-job/SKILL.md`:
- Around line 33-41: Narrow the idempotency check that reuses prior
pr-payload-tests URLs: when querying PR comments via the gh api command shown,
parse and only reuse a found pr-payload-tests.ci.openshift.org/runs/ci/<uuid>
URL if that comment/body includes the current PR head SHA and the same requested
job set; update the matching logic that currently selects any comment containing
"pr-payload-tests" to also verify head SHA and requested job names before
skipping to Step 3 (so you avoid reusing stale runs from prior commits or
different job lists).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ae5f54ef-929e-4d8d-8929-903f89de6586

📥 Commits

Reviewing files that changed from the base of the PR and between be5d46d and 7492d3e.

📒 Files selected for processing (10)
  • .claude-plugin/marketplace.json
  • docs/data.json
  • plugins/ci/.claude-plugin/plugin.json
  • plugins/ci/skills/analyze-payload/SKILL.md
  • plugins/ci/skills/bisect-payload-suspects/SKILL.md
  • plugins/ci/skills/fetch-payloads/SKILL.md
  • plugins/ci/skills/fetch-payloads/fetch_payloads.py
  • plugins/ci/skills/payload-agent/SKILL.md
  • plugins/ci/skills/stage-payload-reverts/SKILL.md
  • plugins/ci/skills/trigger-payload-job/SKILL.md

Comment thread plugins/ci/skills/fetch-payloads/fetch_payloads.py Outdated
Comment thread plugins/ci/skills/payload-agent/SKILL.md
Comment thread plugins/ci/skills/trigger-payload-job/SKILL.md
stbenjam and others added 3 commits March 5, 2026 14:33
Normalize the raw value before iterating: handle None, non-list
(wrap a single string), or unexpected types to avoid TypeError.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When assembling failing_jobs before dispatch, validate that aggregated
jobs have a non-empty underlying_job_name from the subagent analysis.
If missing, record a hard error in the report instead of silently
posting an invalid payload command.

Validation is enforced at two levels:
- payload-agent Step 4: before passing jobs to bisect/stage skills
- trigger-payload-job Step 2: before posting the PR comment

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fetch comments in descending order with per_page=100 and pick the
first match instead of last, so we always get the most recent
openshift-ci[bot] pr-payload-tests comment rather than a stale one.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
plugins/ci/skills/fetch-payloads/fetch_payloads.py (1)

144-146: ⚠️ Potential issue | 🟠 Major

Defensively normalize previousAttemptURLs before iterating.

Line 144 still allows non-list values to flow into Line 145. That can either crash (TypeError) or render garbage (e.g., iterating a string character-by-character).

💡 Proposed fix
-            previous = info.get("previousAttemptURLs") or []
-            for i, prev_url in enumerate(previous, 1):
+            prev_raw = info.get("previousAttemptURLs")
+            if prev_raw is None:
+                previous = []
+            elif isinstance(prev_raw, list):
+                previous = [u for u in prev_raw if isinstance(u, str) and u]
+            elif isinstance(prev_raw, str) and prev_raw:
+                previous = [prev_raw]
+            else:
+                previous = []
+
+            for i, prev_url in enumerate(previous, 1):
                 lines.append(f"            attempt {i}: {prev_url}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/ci/skills/fetch-payloads/fetch_payloads.py` around lines 144 - 146,
Normalize the previousAttemptURLs value before iterating: ensure the value
returned by info.get("previousAttemptURLs") is converted to a list (e.g., map
tuples to lists, wrap a single string/URL into a one-element list, and default
to [] for None or non-iterable types) so the for-loop over previous (and the
lines.append(f"            attempt {i}: {prev_url}") call) cannot iterate over a
raw string or raise TypeError; update the assignment to validate/coerce previous
into a safe list before the for i, prev_url in enumerate(previous, 1) loop.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/ci/skills/trigger-payload-job/SKILL.md`:
- Around line 97-108: The fenced code block showing the PAYLOAD_JOB_RESULT
example lacks a language identifier (triggering markdownlint MD040); update the
fence that surrounds the PAYLOAD_JOB_RESULT sample to use a language tag (e.g.,
replace the leading ``` with ```yaml) so the block is treated as YAML—ensure the
opening fence before "PAYLOAD_JOB_RESULT:" is changed and the closing fence
remains ``` to keep the block valid in SKILL.md.

---

Duplicate comments:
In `@plugins/ci/skills/fetch-payloads/fetch_payloads.py`:
- Around line 144-146: Normalize the previousAttemptURLs value before iterating:
ensure the value returned by info.get("previousAttemptURLs") is converted to a
list (e.g., map tuples to lists, wrap a single string/URL into a one-element
list, and default to [] for None or non-iterable types) so the for-loop over
previous (and the lines.append(f"            attempt {i}: {prev_url}") call)
cannot iterate over a raw string or raise TypeError; update the assignment to
validate/coerce previous into a safe list before the for i, prev_url in
enumerate(previous, 1) loop.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ff02c8ff-a6a6-450d-a971-9ff5c216e96b

📥 Commits

Reviewing files that changed from the base of the PR and between 7492d3e and f76d213.

📒 Files selected for processing (3)
  • plugins/ci/skills/fetch-payloads/fetch_payloads.py
  • plugins/ci/skills/payload-agent/SKILL.md
  • plugins/ci/skills/trigger-payload-job/SKILL.md

Comment thread plugins/ci/skills/trigger-payload-job/SKILL.md

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
plugins/ci/skills/trigger-payload-job/SKILL.md (1)

106-121: ⚠️ Potential issue | 🟡 Minor

Add a language tag to the return-format fenced block.

Line 106 still uses an untyped fence, which continues to trigger MD040.

Suggested patch
-```
+```yaml
 PAYLOAD_JOB_RESULT:
 - pr_url: <the PR URL>
 - comment_url: <URL of the comment posted>
 - payload_test_url: <pr-payload-tests URL, or empty if not found>
 - prow_jobs:
   - job_name: <job name>
     prow_url: <individual prow URL>
   - ...
 - skipped_jobs:
   - job_name: <job name>
     reason: "job trigger limit reached (max 5 non-aggregated, max 1 aggregated per invocation)"
   - ...
 - status: triggered|reused|failed
 - error: none|<description>
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @plugins/ci/skills/trigger-payload-job/SKILL.md around lines 106 - 121, The
fenced example for PAYLOAD_JOB_RESULT is missing a language tag which triggers
MD040; update the triple-backtick fence that precedes the PAYLOAD_JOB_RESULT
block to include a language (e.g., yaml) so the block is typed, leaving the contents unchanged; locate the PAYLOAD_JOB_RESULT example in SKILL.md and change the opening fence to yaml to satisfy the linter.


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @plugins/ci/skills/payload-agent/SKILL.md:

  • Around line 102-107: The doc must state a centralized pre-dispatch allocation
    step so global caps can't be exceeded when dispatch is parallel: update the "Job
    triggering limits" section to require selecting and reserving the allowed slots
    (5 non-aggregated, up to 2 aggregated with the aggregation priority rules)
    across BOTH HIGH and MEDIUM suspects before any parallel dispatch starts;
    describe the allocation algorithm (prioritize higher-confidence suspects, pick
    highest-confidence aggregated candidate first) and note that dispatch should
    only run after slots are reserved and that any jobs not allocated must be
    recorded as "skipped — job trigger limit reached" in the report.

Duplicate comments:
In @plugins/ci/skills/trigger-payload-job/SKILL.md:

  • Around line 106-121: The fenced example for PAYLOAD_JOB_RESULT is missing a
    language tag which triggers MD040; update the triple-backtick fence that
    precedes the PAYLOAD_JOB_RESULT block to include a language (e.g., yaml) so the block is typed, leaving the contents unchanged; locate the PAYLOAD_JOB_RESULT example in SKILL.md and change the opening fence to yaml
    to satisfy the linter.

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Path: .coderabbit.yaml

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `b31798cb-6d2b-4c5d-9d39-ab311124a40e`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between f76d21307f5fa876b608e755f9ecc268498de4c5 and 7dd5d500a548064d9e792f5b99020c94941161d8.

</details>

<details>
<summary>📒 Files selected for processing (3)</summary>

* `plugins/ci/skills/bisect-payload-suspects/SKILL.md`
* `plugins/ci/skills/payload-agent/SKILL.md`
* `plugins/ci/skills/trigger-payload-job/SKILL.md`

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary>

* plugins/ci/skills/bisect-payload-suspects/SKILL.md

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment thread plugins/ci/skills/payload-agent/SKILL.md
stbenjam and others added 3 commits March 6, 2026 09:08
Aggregated jobs only re-run the aggregation analysis on retry, not the
underlying test jobs. Examining previous attempts provides no additional
signal, so instruct subagents to only analyze the most recent attempt
for these jobs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ions

Don't prescribe specific tool names (Task tool, subagent_type) in skill
instructions — let the model choose the appropriate tool for launching
parallel subagents.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Return `no_response` instead of `triggered` when the bot never replies,
  so callers can distinguish between a successful trigger and no response
- Fix YAML result block to use proper mapping syntax with indentation
  instead of top-level list items

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
plugins/ci/skills/analyze-payload/SKILL.md (1)

104-104: Make retry input to subagents deterministic to reduce output variance.

Line 110/114 instructions are directionally correct, but parsing quality will improve if you require a stable format (explicit retry count semantics, ordered URL list, and "none" when empty). This helps keep retry comparisons consistent across parallel subagents.

Suggested prompt tightening
-> Analyze the failure at <prow_url>. This job had <N> retries. The previous attempt URLs are: <previous_attempt_urls>.
+> Analyze the failure at <prow_url>.
+> Retry metadata:
+> - retry_count: <N>  (N = number of previousAttemptURLs)
+> - previous_attempt_urls (oldest→newest): <url1>, <url2>, ... | none

Also applies to: 110-114

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@plugins/ci/skills/analyze-payload/SKILL.md` at line 104, Update the subagent
launch instructions for the Task tool so retry inputs are deterministic: require
an explicit numeric "retry_count" field (0 when none), pass an ordered array
"attempt_urls" sorted oldest→newest, and use the literal string "none" only when
attempt_urls is empty; ensure the text where you instruct to "launch a parallel
subagent" and "Pass the subagent the final Prow URL and all previous attempt
URLs" specifies these exact field names and formats so all subagents receive
stable, comparable retry semantics and URL ordering.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Around line 138-139: The enum for retries_consistent in SKILL.md is missing a
state that represents jobs with zero retries; update the contract for
retries_consistent to include an explicit zero-retry value (e.g., "none" or
"no_retries") alongside the existing "yes", "no", and "only_final_examined", and
update any references to ANALYSIS_RESULT and retry_summary consumers to handle
this new value so downstream parsers aren’t brittle; ensure documentation and
examples in SKILL.md list the new enum value and mention how retry_summary
should be interpreted when retries_consistent == "none".

---

Nitpick comments:
In `@plugins/ci/skills/analyze-payload/SKILL.md`:
- Line 104: Update the subagent launch instructions for the Task tool so retry
inputs are deterministic: require an explicit numeric "retry_count" field (0
when none), pass an ordered array "attempt_urls" sorted oldest→newest, and use
the literal string "none" only when attempt_urls is empty; ensure the text where
you instruct to "launch a parallel subagent" and "Pass the subagent the final
Prow URL and all previous attempt URLs" specifies these exact field names and
formats so all subagents receive stable, comparable retry semantics and URL
ordering.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2253d42f-0239-4ee9-a1f7-8124dd96117f

📥 Commits

Reviewing files that changed from the base of the PR and between 7dd5d50 and 0de1e46.

📒 Files selected for processing (1)
  • plugins/ci/skills/analyze-payload/SKILL.md

Comment thread plugins/ci/skills/analyze-payload/SKILL.md Outdated
Jobs with zero retries (single attempt only) now return
retries_consistent: no_retries instead of leaving the field ambiguous.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@stbenjam stbenjam changed the title Extract trigger-payload-job skill and surface retry attempt URLs Payload analysis skill improvements Mar 6, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
plugins/ci/skills/trigger-payload-job/SKILL.md (1)

106-121: ⚠️ Potential issue | 🟡 Minor

Add a language identifier to the return-format fenced block.

The opening fence is still untyped, which triggers MD040 and reduces markdown/tooling consistency.

Suggested patch
-```
+```yaml
 PAYLOAD_JOB_RESULT:
   pr_url: <the PR URL>
   comment_url: <URL of the comment posted>
   payload_test_url: <pr-payload-tests URL, or empty if not found>
   prow_jobs:
     - job_name: <job name>
       prow_url: <individual prow URL>
     - ...
   skipped_jobs:
     - job_name: <job name>
       reason: "job trigger limit reached (max 5 non-aggregated, max 1 aggregated per invocation)"
     - ...
   status: triggered|reused|no_response|failed
   error: none|<description>
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @plugins/ci/skills/trigger-payload-job/SKILL.md around lines 106 - 121, The
fenced code block that documents the PAYLOAD_JOB_RESULT return format is missing
a language identifier and triggers MD040; update the opening fence for that
block to include a language (e.g., add "yaml" after the backticks) so the block
becomes a typed fenced code block (reference the PAYLOAD_JOB_RESULT example
block in SKILL.md and the opening ``` fence).


</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @plugins/ci/skills/analyze-payload/SKILL.md:

  • Around line 138-139: The YAML enum values for
    ANALYSIS_RESULT.retries_consistent are ambiguous (yes/no) and must be replaced
    with explicit string-safe tokens; update the SKILL.md definition for
    retries_consistent to use unambiguous literals (e.g.,
    "consistent"/"inconsistent" or keep "no_retries"/"only_final_examined") instead
    of bare yes/no, and then update any code that reads
    ANALYSIS_RESULT.retries_consistent (search for references to ANALYSIS_RESULT and
    retries_consistent) to accept the new string values and treat them strictly as
    strings (not booleans) when serializing/deserializing YAML and validating enums.

Duplicate comments:
In @plugins/ci/skills/trigger-payload-job/SKILL.md:

  • Around line 106-121: The fenced code block that documents the
    PAYLOAD_JOB_RESULT return format is missing a language identifier and triggers
    MD040; update the opening fence for that block to include a language (e.g., add
    "yaml" after the backticks) so the block becomes a typed fenced code block
    (reference the PAYLOAD_JOB_RESULT example block in SKILL.md and the opening ```
    fence).

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Path: .coderabbit.yaml

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `b2e3e234-99e3-4955-889e-fc601f4f3618`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 0de1e4684a4bf368f847c7a530d965c017eef4d8 and bf9710e06a554a76a3f4cb723483a47c4b6948df.

</details>

<details>
<summary>📒 Files selected for processing (4)</summary>

* `plugins/ci/skills/analyze-payload/SKILL.md`
* `plugins/ci/skills/bisect-payload-suspects/SKILL.md`
* `plugins/ci/skills/stage-payload-reverts/SKILL.md`
* `plugins/ci/skills/trigger-payload-job/SKILL.md`

</details>

<details>
<summary>🚧 Files skipped from review as they are similar to previous changes (1)</summary>

* plugins/ci/skills/stage-payload-reverts/SKILL.md

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Comment thread plugins/ci/skills/analyze-payload/SKILL.md
stbenjam and others added 2 commits March 6, 2026 09:30
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Each F/S marker in the blocking jobs history column should link to the
corresponding Prow job URL from that payload, making it easy to inspect
any run directly from the summary table.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@dgoodwin dgoodwin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Mar 6, 2026
@openshift-ci

openshift-ci Bot commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dgoodwin, stbenjam

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit 2739d88 into openshift-eng:main Mar 6, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants