Add PR payload triage and job discovery to yolo-agent#213
Conversation
- New payload_job_discovery.py: auto-discover correct payload jobs from PR changed files by cross-referencing pipeline_run_if_changed regexes in the release repo CI configs against nightly job definitions - New pr_payload.py: diff-aware triage classifier that fetches payload run results, deep junit artifacts, and PR diff to classify failures as pr-caused vs flaky - New pr-payload.sh: three-phase shell wrapper (discovery → validation → triage) with staleness detection comparing payload comment timestamps against latest commit - Updated __main__.py with --discover and --pr-payload-url CLI modes - Updated SKILL.md with Payload Track handling for triage, discovery, and stale result modes — always asks before posting /payload-job Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Consolidate 5 sequential jq calls into 1 in pr-payload.sh - Deduplicate _parse_pr_ref by importing from payload_job_discovery - Remove hardcoded tnf-dev-env path from find_release_repo() Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
When the payload page returns 200 but has no Prow job links (expired run), fall through to discovery mode instead of dying with exit 3. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lucaconsalvi 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 |
openshift-ci
Bot
added
the
approved
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR adds payload-job discovery and PR payload triage to payload-monitor, exposes both through the CLI and shell wrapper, and updates workflow docs plus plugin metadata for the new payload track. ChangesPayload workflow
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)plugins/pr-review/skills/yolo-agent/SKILL.mdmarkdownlint-cli2 v0.22.1 (markdownlint v0.40.0) ... [truncated 1092 characters] ... node:internal/modules/esm/resolve:271:11) Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/pr-review/skills/yolo-agent/SKILL.md (1)
138-152: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
"PR is ready"never becomes reachable for payload-matching PRs
pr-payload.shkeeps returningdiscoveryfor PRs whose changed files match a payload job, ortriageonce a payload URL exists. Because step 2b only completes when payload mode is"none", payload-relevant PRs stay in the 2c loop and never hit thecompletestate. If that’s intentional, document the manual exit path; otherwise add a completion branch for payload flows.🤖 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 `@plugins/pr-review/skills/yolo-agent/SKILL.md` around lines 138 - 152, The completion path in the yolo-agent flow is unreachable for payload-matching PRs because step 2b only allows `"PR is ready"` when payload mode from step 2a is `"none"`. Update the decision logic around the 2b/2c branches so payload-relevant PRs can still reach `complete` when CI is green and there are no new or resurfaced comments, or explicitly document the intended manual exit path. Use the existing step labels and the `pr-payload.sh` payload mode handling to locate the branching logic.
🧹 Nitpick comments (5)
payload-monitor/payload_monitor/models.py (1)
286-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
Literaltyping forverdictfield.The comment documents allowed values ("pr-caused" | "flaky" | "unknown") but the field is a plain
str. UsingLiteral["pr-caused", "flaky", "unknown"]would let static analysis (mypy/pyright) catch typos at the source instead of relying on the comment.♻️ Proposed refactor
+from typing import Literal + `@dataclass` class FailureVerdict: - verdict: str # "pr-caused" | "flaky" | "unknown" + verdict: Literal["pr-caused", "flaky", "unknown"] reason: str matched_files: list[str] = field(default_factory=list)🤖 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 `@payload-monitor/payload_monitor/models.py` around lines 286 - 289, Update the FailureVerdict model so verdict uses a Literal type instead of a plain str, matching the documented allowed values pr-caused, flaky, and unknown. Adjust the FailureVerdict class definition in models.py to import and apply Literal to the verdict field while keeping reason and matched_files unchanged, so static type checkers can catch invalid verdict values.payload-monitor/payload_monitor/collectors/payload_job_discovery.py (1)
80-92: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSilent fallback to
"main"onghfailure hides real errors.If
gh pr viewfails (non-zero exit, auth error, network issue) or returns empty output, the function silently returns"main"with no log line. This can silently mis-detect the branch/version downstream, producing a confusing "No CI config found" error rather than surfacing the actualghfailure.♻️ Proposed fix
try: result = subprocess.run( ["gh", "pr", "view", number, "--repo", f"{org}/{repo}", "--json", "baseRefName", "--jq", ".baseRefName"], capture_output=True, text=True, timeout=15, ) if result.returncode == 0 and result.stdout.strip(): return result.stdout.strip() + logger.warning(f"gh pr view failed for {org}/{repo}#{number}: {result.stderr.strip()}") except (subprocess.TimeoutExpired, FileNotFoundError): - pass + logger.warning(f"gh pr view timed out or gh not found for {org}/{repo}#{number}") return "main"🤖 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 `@payload-monitor/payload_monitor/collectors/payload_job_discovery.py` around lines 80 - 92, The _get_pr_branch helper is silently falling back to "main" when gh pr view fails or returns empty output, which hides the real problem. Update _get_pr_branch in payload_job_discovery.py to log the gh failure details (non-zero exit, stderr, timeout, or missing output) before returning the default, and make sure the fallback is only used after recording enough context to diagnose the branch lookup issue.payload-monitor/payload_monitor/collectors/pr_payload.py (2)
94-136: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winParsing untrusted junit XML with
xml.etree.ElementTreeinstead ofdefusedxml.
_fetch_deep_junit_failuresparses XML fetched from CI artifacts (produced by test code that may include PR-authored content) with the standard libraryET.fromstring, which is susceptible to entity-expansion DoS. Preferdefusedxml.ElementTree.fromstringas a drop-in, safer replacement.🛡️ Proposed fix
-import xml.etree.ElementTree as ET +import defusedxml.ElementTree as ETPlease confirm
defusedxmlis an acceptable/available dependency for this package before adopting it.🤖 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 `@payload-monitor/payload_monitor/collectors/pr_payload.py` around lines 94 - 136, The XML parsing in _fetch_deep_junit_failures currently uses xml.etree.ElementTree.fromstring on CI-provided junit artifacts, which should be switched to the safer defusedxml.ElementTree equivalent. Update the imports and parsing call in _fetch_deep_junit_failures to use defusedxml as a drop-in replacement, and first verify that defusedxml is already available for payload_monitor before making the change.Source: Linters/SAST tools
204-211: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
pr_payload._parse_pr_refshadowspayload_job_discovery._parse_pr_refwith a different return shape.Two different modules export a same-named
_parse_pr_ref(one returns(org, repo, number), this one returns(repo, number)). The alias_parse_pr_ref_3on import (line 21) already signals this friction. Renaming this wrapper (e.g._parse_repo_and_number) would remove the ambiguity for future readers/importers.🤖 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 `@payload-monitor/payload_monitor/collectors/pr_payload.py` around lines 204 - 211, The local pr_payload._parse_pr_ref wrapper is confusing because it shadows payload_job_discovery._parse_pr_ref while returning a different shape. Rename this wrapper to something explicit like _parse_repo_and_number, update any internal callers in pr_payload.py, and keep using _parse_pr_ref_3 only as the underlying parser so the repo/number return shape is obvious and the name collision is removed.plugins/pr-review/scripts/pr-payload.sh (1)
66-71: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSimplify per shellcheck (SC2005).
echo "$(cd "${candidate}" && pwd)"is an unnecessary echo-of-command-substitution.♻️ Proposed fix
- if [[ -d "${candidate}" ]]; then - echo "$(cd "${candidate}" && pwd)" - return - fi + if [[ -d "${candidate}" ]]; then + (cd "${candidate}" && pwd) + return + fi🤖 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 `@plugins/pr-review/scripts/pr-payload.sh` around lines 66 - 71, The candidate path resolution in the shell loop uses an unnecessary echo around command substitution, triggering SC2005. Update the logic in the candidate search block to print the resolved directory directly from the subshell output instead of wrapping it in echo, while keeping the existing behavior in the candidate iteration and directory check intact.Source: Linters/SAST tools
🤖 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 `@payload-monitor/payload_monitor/__main__.py`:
- Around line 111-141: The discover path in __main__.py does not treat
discover_payload_jobs business failures as process failures, so errors inside
DiscoveryResult are printed and the command exits 0. Update the discover branch
around discover_payload_jobs/format_discovery_json/format_discovery_markdown to
detect when the returned result has .error set, log or print that error, and
exit non-zero instead of continuing normally. Preserve the existing success path
when no error is present.
- Around line 143-175: The PR payload triage path in __main__.py lets
fetch_pr_payload_run() exceptions escape and show a traceback. Wrap the
fetch_pr_payload_run(pr_payload_url) call in a targeted try/except, log a clear
CLI error for 4xx/5xx or stale/expired payload URLs, and exit cleanly with
SystemExit(1) before continuing to fetch_pr_changed_files, fetch_pr_diff, or
build_triage_result.
In `@payload-monitor/payload_monitor/collectors/pr_payload.py`:
- Line 76: The list comprehension in the PR payload collector uses an ambiguous
single-letter variable name, which triggers the ruff E741 rule. Update the
comprehension in the code that processes result.stdout in the PR payload
collector to use a descriptive name instead of l, keeping the same stripping and
filtering behavior. Reference the relevant result.stdout parsing logic so the
variable rename is applied consistently.
- Around line 160-182: The PR payload fetch flow in fetch_pr_payload_run
currently lets resp.raise_for_status() abort on expired or stale pages, so
handle requests.RequestException around the initial
_session.get/raise_for_status path and return an empty list instead of crashing.
Keep the fallback behavior aligned with the existing logging in
fetch_pr_payload_run and the caller in __main__.py so --pr-payload-url fails
cleanly when the payload page is no longer valid.
In `@plugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.md`:
- Around line 51-59: The Follow-up Actions guidance in SKILL.md should
explicitly require user confirmation before any `/payload-job` re-post, since
the current wording only says to suggest re-posting the original comment
verbatim. Update the instructions around the “All FLAKY” path and the
`/payload-job` note to say the agent must ask the user before posting and must
not auto-post or assume permission. Reference the “Follow-up Actions” section
and the `/payload-job` re-trigger note so the confirmation requirement is
unambiguous.
- Around line 1-6: The SKILL.md frontmatter for edge-ocp-ci:pr-payload-triage is
missing the required allowed-tools entry for a user-invocable skill that runs a
Bash command. Update the frontmatter near the name/description/user-invocable
fields to include a minimal allowed-tools list that explicitly declares Bash,
and keep it aligned with the skill’s shell-based payload_monitor invocation.
- Around line 21-42: The SKILL.md Execution/Output guidance only describes the
happy path and is missing failure-mode handling. Update the sections around the
payload-monitor flow to document what to do when the pr-payload-tests page is
expired/unavailable, when payload-monitor or its fetch/classification steps
error out, and when gh/gsutil authentication or access checks fail. Add explicit
guard/check instructions between the fetch, GCS lookup, junit download, GitHub
diff fetch, and report steps, and reference the payload-monitor CLI flow and
markdown triage report output so the edge-case handling is easy to find.
- Around line 44-49: The rendered markdown table in the Interpreting Results
section uses emoji icons, which violates the no-emoji rule for regular markdown
content. Update the verdict examples in SKILL.md so the table entries referenced
by the interpreting-results guidance use non-emoji text labels or symbols that
comply with the markdown style rules, and keep the table professional and terse.
In `@plugins/pr-review/scripts/pr-payload.sh`:
- Around line 159-188: The /payload-job validation in the existing_cmd block is
using short suggestion_names with a substring test against job_name, which can
misclassify unrelated jobs; switch the matching logic to compare against the
unique periodic_name values from discovery_json instead of as_name, and keep the
validation_json note based on that exact match. Also update the job_name
extraction in the same flow to use shell parameter expansion instead of the sed
command for the /payload-job prefix removal.
In `@plugins/pr-review/skills/yolo-agent/SKILL.md`:
- Around line 277-284: The non-stale re-trigger path in SKILL.md is missing the
“ask before posting” confirmation guard, so update the guidance around the
stale:false handling to match the safer behavior used elsewhere. In the section
that references checking stale first and posting the exact trigger_command,
explicitly require asking the user before posting any PR comment and forbid
auto-posting, consistent with the stale:true and discovery-mode instructions.
Keep the wording aligned with the existing triage/report flow so the
authoritative verdict handling in the stale:false branch is unchanged.
---
Outside diff comments:
In `@plugins/pr-review/skills/yolo-agent/SKILL.md`:
- Around line 138-152: The completion path in the yolo-agent flow is unreachable
for payload-matching PRs because step 2b only allows `"PR is ready"` when
payload mode from step 2a is `"none"`. Update the decision logic around the
2b/2c branches so payload-relevant PRs can still reach `complete` when CI is
green and there are no new or resurfaced comments, or explicitly document the
intended manual exit path. Use the existing step labels and the `pr-payload.sh`
payload mode handling to locate the branching logic.
---
Nitpick comments:
In `@payload-monitor/payload_monitor/collectors/payload_job_discovery.py`:
- Around line 80-92: The _get_pr_branch helper is silently falling back to
"main" when gh pr view fails or returns empty output, which hides the real
problem. Update _get_pr_branch in payload_job_discovery.py to log the gh failure
details (non-zero exit, stderr, timeout, or missing output) before returning the
default, and make sure the fallback is only used after recording enough context
to diagnose the branch lookup issue.
In `@payload-monitor/payload_monitor/collectors/pr_payload.py`:
- Around line 94-136: The XML parsing in _fetch_deep_junit_failures currently
uses xml.etree.ElementTree.fromstring on CI-provided junit artifacts, which
should be switched to the safer defusedxml.ElementTree equivalent. Update the
imports and parsing call in _fetch_deep_junit_failures to use defusedxml as a
drop-in replacement, and first verify that defusedxml is already available for
payload_monitor before making the change.
- Around line 204-211: The local pr_payload._parse_pr_ref wrapper is confusing
because it shadows payload_job_discovery._parse_pr_ref while returning a
different shape. Rename this wrapper to something explicit like
_parse_repo_and_number, update any internal callers in pr_payload.py, and keep
using _parse_pr_ref_3 only as the underlying parser so the repo/number return
shape is obvious and the name collision is removed.
In `@payload-monitor/payload_monitor/models.py`:
- Around line 286-289: Update the FailureVerdict model so verdict uses a Literal
type instead of a plain str, matching the documented allowed values pr-caused,
flaky, and unknown. Adjust the FailureVerdict class definition in models.py to
import and apply Literal to the verdict field while keeping reason and
matched_files unchanged, so static type checkers can catch invalid verdict
values.
In `@plugins/pr-review/scripts/pr-payload.sh`:
- Around line 66-71: The candidate path resolution in the shell loop uses an
unnecessary echo around command substitution, triggering SC2005. Update the
logic in the candidate search block to print the resolved directory directly
from the subshell output instead of wrapping it in echo, while keeping the
existing behavior in the candidate iteration and directory check intact.
🪄 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: dced00b1-459f-4b32-954f-5360773cd83d
📒 Files selected for processing (9)
payload-monitor/payload_monitor/__main__.pypayload-monitor/payload_monitor/collectors/payload_job_discovery.pypayload-monitor/payload_monitor/collectors/pr_payload.pypayload-monitor/payload_monitor/models.pypayload-monitor/tests/test_collectors_payload_job_discovery.pypayload-monitor/tests/test_collectors_pr_payload.pyplugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.mdplugins/pr-review/scripts/pr-payload.shplugins/pr-review/skills/yolo-agent/SKILL.md
…necessary echo (SC2005)
…ps, and validation fixes
coderabbitai
Bot
added
the
ready-for-human-review
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/pr-review/skills/yolo-agent/SKILL.md (1)
282-284: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReword the fragmented trailing clause for clarity.
The confirmation guard is a correct fix, but "NOT just
/payload-joband NOT/retest" now dangles as a disconnected fragment after "Never auto-post," making it unclear what it modifies.📝 Proposed fix
-authoritative. When suggesting a re-trigger, ask the user to confirm -before posting the exact `trigger_command` string as a PR comment. -Never auto-post. NOT just `/payload-job` and NOT `/retest`. +authoritative. When suggesting a re-trigger, ask the user to confirm +before posting the exact `trigger_command` string as a PR comment — +never auto-post, and never substitute `/payload-job` or `/retest` +for the exact command.As per coding guidelines: "Markdown must be professional, terse, customer-centric — no emojis or filler."
🤖 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 `@plugins/pr-review/skills/yolo-agent/SKILL.md` around lines 282 - 284, The trailing restriction in the re-trigger guidance is fragmented and should be rewritten for clarity in SKILL.md. Update the guidance around the confirmation guard and the exact `trigger_command` wording so the “NOT just `/payload-job` and NOT `/retest`” requirement is grammatically attached to the instruction about posting the comment, rather than left as a dangling clause; keep the wording concise and professional in the same section.Source: Coding guidelines
🤖 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 `@payload-monitor/payload_monitor/__main__.py`:
- Around line 162-169: The `fetch_pr_payload_run` error path in `__main__.py` is
re-raising `SystemExit` without preserving the caught
`requests.RequestException` context. Update the `except
requests.RequestException as exc` block so the `raise SystemExit(1)` is chained
from `exc`, keeping the original cause visible while leaving the `click.echo`
and `logger.error` behavior unchanged.
---
Nitpick comments:
In `@plugins/pr-review/skills/yolo-agent/SKILL.md`:
- Around line 282-284: The trailing restriction in the re-trigger guidance is
fragmented and should be rewritten for clarity in SKILL.md. Update the guidance
around the confirmation guard and the exact `trigger_command` wording so the
“NOT just `/payload-job` and NOT `/retest`” requirement is grammatically
attached to the instruction about posting the comment, rather than left as a
dangling clause; keep the wording concise and professional in the same section.
🪄 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: 64038a0c-2086-4839-a74b-d3a9bb08844d
📒 Files selected for processing (9)
.claude-plugin/marketplace.jsonpayload-monitor/payload_monitor/__main__.pypayload-monitor/payload_monitor/collectors/payload_job_discovery.pypayload-monitor/payload_monitor/collectors/pr_payload.pyplugins/edge-ocp-ci/.claude-plugin/plugin.jsonplugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.mdplugins/pr-review/.claude-plugin/plugin.jsonplugins/pr-review/scripts/pr-payload.shplugins/pr-review/skills/yolo-agent/SKILL.md
✅ Files skipped from review due to trivial changes (4)
- plugins/edge-ocp-ci/.claude-plugin/plugin.json
- .claude-plugin/marketplace.json
- plugins/pr-review/.claude-plugin/plugin.json
- plugins/edge-ocp-ci/skills/pr-payload-triage/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (3)
- payload-monitor/payload_monitor/collectors/payload_job_discovery.py
- payload-monitor/payload_monitor/collectors/pr_payload.py
- plugins/pr-review/scripts/pr-payload.sh
coderabbitai
Bot
removed
the
ready-for-human-review
openshift-ci
Bot
added
the
ready-for-human-review
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/pr-review/skills/yolo-agent/SKILL.md (2)
332-343: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire explicit confirmation before any push.
The
--yolopath still auto-applies changes and batches them forpr-push.sh, which gives the agent a branch-mutating action without human approval.Proposed fix
- - Trivial changes: apply directly, batch for auto-push - - Non-trivial changes: if `--yolo` is active, apply directly and batch for auto-push (same as trivial). Otherwise, display diff and ask for confirmation + - Trivial changes: apply directly, then ask for confirmation before `pr-push.sh` + - Non-trivial changes: if `--yolo` is active, apply directly, then ask for confirmation before `pr-push.sh`. Otherwise, display diff and ask for confirmationAs per path instructions, side effects safety: explicitly declare side effects/read-only, require user confirmation for destructive actions.
🤖 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 `@plugins/pr-review/skills/yolo-agent/SKILL.md` around lines 332 - 343, Update the workflow in SKILL.md so push actions are never automatic: in the logic around `--yolo`, `pr-push.sh`, and the “On successful push” section, require explicit user confirmation before any branch-mutating step, including applying non-trivial changes and batching for push. Make the side-effect policy explicit by separating read-only guidance from destructive actions, and ensure the confirmation gate is checked before calling `pr-push.sh` regardless of `--yolo`.Sources: Path instructions, Linters/SAST tools
138-157: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExclude stale triage from the ready-label branch.
A completed triage run with zeropr-causedverdicts can still be stale after a newer commit, and this path would auto-post/label ready-for-human-reviewfor obsolete results. Requirestale: falsehere.🤖 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 `@plugins/pr-review/skills/yolo-agent/SKILL.md` around lines 138 - 157, The ready-label branch in the yolo-agent flow is missing a freshness check, so completed triage results can still trigger a stale /label ready-for-human-review comment after newer commits. Update the condition in the decision logic around the payload/triage checks to also require stale: false before posting the PR comment and marking the run complete, keeping the existing checks for mode, complete, and zero pr-caused verdicts.
🧹 Nitpick comments (1)
plugins/pr-review/skills/yolo-agent/SKILL.md (1)
380-385: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInline the failure policy in this step.
This step delegates reply/thread failure handling to
${PLUGIN_DIR}/references/error-handling.md, which makes the recovery path harder to follow when the agent is already handling an error.Based on learnings, co-locate failure policies and edge-case rules inline with the specific step that needs them.
🤖 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 `@plugins/pr-review/skills/yolo-agent/SKILL.md` around lines 380 - 385, Inline the reply/thread failure policy directly in the “Reply failure handling” step instead of delegating to `${PLUGIN_DIR}/references/error-handling.md`. Update the guidance near the reply handling section in `SKILL.md` so it explicitly states the non-fatal retry-tracked behavior, 3-strike escalation, and thread-resolution failure handling rules in place, keeping the recovery path co-located with the step that uses it.Source: Learnings
🤖 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.
Outside diff comments:
In `@plugins/pr-review/skills/yolo-agent/SKILL.md`:
- Around line 332-343: Update the workflow in SKILL.md so push actions are never
automatic: in the logic around `--yolo`, `pr-push.sh`, and the “On successful
push” section, require explicit user confirmation before any branch-mutating
step, including applying non-trivial changes and batching for push. Make the
side-effect policy explicit by separating read-only guidance from destructive
actions, and ensure the confirmation gate is checked before calling `pr-push.sh`
regardless of `--yolo`.
- Around line 138-157: The ready-label branch in the yolo-agent flow is missing
a freshness check, so completed triage results can still trigger a stale /label
ready-for-human-review comment after newer commits. Update the condition in the
decision logic around the payload/triage checks to also require stale: false
before posting the PR comment and marking the run complete, keeping the existing
checks for mode, complete, and zero pr-caused verdicts.
---
Nitpick comments:
In `@plugins/pr-review/skills/yolo-agent/SKILL.md`:
- Around line 380-385: Inline the reply/thread failure policy directly in the
“Reply failure handling” step instead of delegating to
`${PLUGIN_DIR}/references/error-handling.md`. Update the guidance near the reply
handling section in `SKILL.md` so it explicitly states the non-fatal
retry-tracked behavior, 3-strike escalation, and thread-resolution failure
handling rules in place, keeping the recovery path co-located with the step that
uses it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f6da322d-bd00-453b-baf7-d4ebe12bc640
📒 Files selected for processing (1)
plugins/pr-review/skills/yolo-agent/SKILL.md
|
/label ready-for-human-review This message is AI generated by the yolo-agent of edge-tooling. |
1 participant
Summary
Adds an AI-powered PR payload CI triage pipeline to the yolo-agent, covering three
phases:
files by cross-referencing pipeline_run_if_changed regexes in openshift/release CI
configs against nightly job definitions. Works for any repo (origin, installer,
MCO, etc.)
classifies each failure as pr-caused vs flaky based on whether the failing test
appears in the code changes
commit. If a new commit was pushed after the payload was triggered, marks results
as stale and suggests re-triggering with the correct command
Design decisions
Summary by CodeRabbit
payload-monitorCLI modes for payload job discovery and triage, withmarkdown/jsonoutput./payload-jobtriggers, and flags stale payload runs.