TRT-2612: feat(ci): teach payload-analysis to read RPM changelogs as evidence - #689
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: petr-muller 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 |
WalkthroughThe CI plugin version changes from 0.0.82 to 0.0.83. The payload-analysis skill adds RPM changelog collection, correlation, reporting, and completeness checks for RHCOS investigations. ChangesPayload analysis updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 9 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (9 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
|
|
||
| 1. **Inline in `summary.json`** — `rpm_changelogs[]` at the top level. The baseline entry (the one with `is_baseline: true`) carries its content inline under `diff`: `diff.changed[]` with `package`, `old`, `new`, and `changelog` (the entries the new version added); `diff.added[]` / `diff.removed[]` with `package` and `version`. This covers the full target-vs-baseline diff without a file read. | ||
|
|
||
| 2. **Per-hop report files** — `rpm_changelogs[]` entries also have a `changelogs` path pointing at `<target-tag>/rpm-changelogs/<variant>/<older-tag>.md`. Read these to find which intermediate payload introduced a specific package bump (useful when the originating payload for a failure is not the baseline). Note that `payloads[]` entries for the target payload and for payloads whose RPMDBs could not be extracted have no `rpm_changelogs[]` at all — the field is absent, not empty. An intermediate hop where the RHCOS image did not change shows `changed: 0, added: 0, removed: 0`; use this to pinpoint which hop introduced a given RHCOS bump. |
There was a problem hiding this comment.
content-weak-language): Weak language (non-actionable): 'Note that' — Restructure — state the constraint directly
| > | ||
| > **RHCOS RPM changes**: Read `<summary_json_path>` and find the entry in `payloads[]` whose `tag` equals `<originating_payload_tag>`. If that entry has an `rhcos_changes[]` array, look up the RHCOS variant matching this job's `rhcos_version` using the tag mapping: `rhel-coreos` → `rhcos9`/`rhcos9-default`, `rhel-coreos-10` → `rhcos10`/`rhcos10-default`, both apply to `rhcos9_10`. Check whether any changed, added, or removed RPM packages overlap with the failure's root cause. If the failure involves OS-level components (kernel, bootloader, systemd, SELinux, rpm-ostree, cri-o, crun, runc, networking) and matching packages changed, note the potential correlation in your ANALYSIS_RESULT. | ||
| > | ||
| > **RHCOS RPM changelogs**: When you find a suspect RPM package via `rhcos_changes[]`, read the changelog text to see *what* actually changed in that package — use it as evidence to support or rule out the package as the regression culprit. To read the baseline changelog: in `<summary_json_path>`, find `rpm_changelogs[]` at the top level → the entry with `is_baseline: true` for the matching variant — its `diff.changed[]` entries each have a `changelog` field with the entries the new version added. For the originating payload's hop specifically: find `rpm_changelogs[]` in `payloads[]` for `<originating_payload_tag>` matching the variant, and read the file at its `changelogs` path (relative to `<snapshot_dir>`). Note that multiple binary RPMs built from the same source RPM share identical changelogs — when several packages have the same version bump and changelog, read it once and treat them as a single change. A changelog entry that describes a change in the subsystem or behavior seen in the failure is strong evidence; a changelog about an unrelated subsystem rules the package out even though its version changed. |
There was a problem hiding this comment.
content-weak-language): Weak language (non-actionable): 'Note that' — Restructure — state the constraint directly
|
/hold |
|
/test eval-payload-analysis-changed |
|
@petr-muller: This pull request references TRT-2612 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@plugins/ci/skills/payload-analysis/SKILL.md`:
- Around line 788-789: Update the completeness reviewer inputs near the
suspect-record review to include the source changelog contents from each
relevant `rpm_changelogs[].changelogs` file, preferably as a normalized evidence
map keyed to the originating hop or package. Preserve the existing suspect
records and ensure the reviewer can compare each `changelog_evidence` value with
its source entry.
- Around line 285-286: Update the RHCOS RPM changelog analysis instructions
around rpm_changelogs[] to explicitly handle absent data and missing matching
entries, changelog paths, or changelog fields by requiring
rhcos_rpm_changelog_evidence: unavailable. Reserve rhcos_rpm_changelog_evidence:
none only for changelogs that were successfully read and contained no relevant
entry.
- Around line 632-641: HTML-escape each changelog entry before interpolating it
into the `changelog-evidence` `<pre>` block in the RPM Changelog Evidence
template. Preserve the displayed changelog text while ensuring crafted entries
cannot alter report markup or execute script.
- Around line 452-465: Align the RHCOS suspect schema by adding
changelog_evidence to payload-results-yaml/SKILL.md and validate.py, enforcing
string values with the allowed sentinels "none" and "unavailable". Add
regression coverage for validation and preserve multiline evidence through safe
YAML serialization. Review payload-autodl-json’s existing contract and retain
its current fields unless the schema requires this evidence.
- Around line 251-266: Consolidate the RPM changelog workflow documentation in
this section with the corresponding implementation from PR 688, removing
duplicated or conflicting guidance while preserving the complete schema and
analysis behavior. Leave unrelated content from PR 617 out of this change, and
incorporate only the complementary producer/schema details from
payload-snapshot/SKILL.md where needed.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: cb4d13bb-5a61-4255-8ed2-af1ca98cd55c
📒 Files selected for processing (4)
.claude-plugin/marketplace.jsondocs/index.htmlplugins/ci/.claude-plugin/plugin.jsonplugins/ci/skills/payload-analysis/SKILL.md
| **Use RPM changelog text to sharpen correlation.** When a package is flagged as a suspect based on version change alone, read its changelog entries (Step 3.7b) before finalizing the correlation level. A changelog entry that describes a change in the exact subsystem or behavior seen in the failure elevates `possible` to `likely`. Conversely, a changelog that only describes unrelated changes downgrades the suspicion — the version bump is coincidental. Record the relevant changelog entry in the suspect's `rationale` when it provides evidence. | ||
|
|
||
| **Subpackage dedup.** Multiple binary RPMs built from the same source RPM share identical changelogs. When several packages have the same version bump and the same changelog, record one suspect and cite the relevant changelog entry — do not list each subpackage as a separate suspect. | ||
|
|
||
| **Pinpointing which hop introduced the RHCOS change.** The top-level `rpm_changelogs[]` entries include intermediate hops — some may show `changed: 0` (no RPM changes between those two payloads), while others show the actual bump. When the originating payload for a failure mode differs from the baseline, check the intermediate hop's changelog to confirm the RPM change landed in that specific hop, not earlier. This narrows the timing correlation. | ||
|
|
||
| For each RHCOS RPM suspect, record: | ||
| - `rhcos_tag`: the RHCOS image stream tag (e.g., `rhel-coreos-10`) | ||
| - `rhcos_name`: human-readable name (e.g., "Red Hat Enterprise Linux CoreOS 10.2") | ||
| - `package`: the RPM package name | ||
| - `old_version`, `new_version`: the version change | ||
| - `changelog_evidence`: the specific changelog entry or entries that relate to the failure (verbatim text from the RPM changelog diff), or `"none"` if the changelog does not contain entries relevant to the failure mode | ||
| - `failing_jobs`: list of job names where this package change may be relevant | ||
| - `rationale`: why this package is suspected (e.g., "systemd update correlates with variant-isolated boot timeout in RHCOS 10 jobs") | ||
| - `rationale`: why this package is suspected — cite the changelog entry when it provides evidence (e.g., "systemd 256.4→256.7 changelog includes 'fix race condition in mount unit ordering' — correlates with mount timeout in RHCOS 10 jobs") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== candidate files ==="
git ls-files | rg '(^|/)(SKILL\.md|validate\.py|payload-autodl-json|payload-results-yaml|payload-analysis)' | head -200
echo "=== skill structure and relevant sections ==="
SKILL=$(fd -t f '^SKILL\.md$' plugins/ci/skills | rg '/payload-analysis/' | head -1)
echo "SKILL=$SKILL"
ast-grep outline "$SKILL" | head -200
sed -n '430,475p' "$SKILL"
sed -n '610,655p' "$SKILL"
sed -n '770,805p' "$SKILL"
echo "=== validator candidates ==="
fd -t f 'validate\.py$' plugins/ci/skills | sort
VALIDATOR=$(fd -t f '^validate\.py$' plugins/ci/skills | rg '/payload-results-yaml/scripts/' | head -1)
echo "VALIDATOR=$VALIDATOR"
if [ -n "$VALIDATOR" ]; then
wc -l "$VALIDATOR"
ast-grep outline "$VALIDATOR" | head -200
rg -n -C 8 'rhcos_tag|failing_jobs|changelog_evidence|suspect|required|yaml|dump|safe' "$VALIDATOR"
fi
echo "=== payload-autodl-json references ==="
rg -n -C 8 'payload-autodl-json|RHCOS|rhcos_tag|changelog|suspicion|package list|package' plugins/ci/skills agents plugins 2>/dev/null | head -300Repository: openshift-eng/ai-helpers
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== candidate files ==="
git ls-files | rg '(^|/)(SKILL\.md|validate\.py|payload-autodl-json|payload-results-yaml|payload-analysis)' | head -200
echo "=== skill sections ==="
SKILL=$(fd -t f '^SKILL\.md$' plugins/ci/skills | rg '/payload-analysis/' | head -1)
echo "SKILL=$SKILL"
sed -n '430,475p' "$SKILL"
sed -n '610,655p' "$SKILL"
sed -n '770,805p' "$SKILL"
echo "=== validator ==="
VALIDATOR=$(fd -t f '^validate\.py$' plugins/ci/skills | rg '/payload-results-yaml/scripts/' | head -1)
echo "VALIDATOR=$VALIDATOR"
wc -l "$VALIDATOR"
rg -n -C 8 'rhcos_tag|failing_jobs|changelog_evidence|suspect|required|yaml|dump|safe' "$VALIDATOR"
echo "=== payload-autodl-json references ==="
rg -n -C 8 'payload-autodl-json|RHCOS|rhcos_tag|changelog|suspicion|package' plugins/ci/skills agents 2>/dev/null | head -300Repository: openshift-eng/ai-helpers
Length of output: 50382
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== changed-file summary ==="
git diff --stat
git status --short
echo "=== payload-results-yaml schema ==="
sed -n '1,115p' plugins/ci/skills/payload-results-yaml/SKILL.md
sed -n '175,205p' plugins/ci/skills/payload-results-yaml/SKILL.md
echo "=== payload-autodl-json schema and validator ==="
sed -n '1,125p' plugins/ci/skills/payload-autodl-json/SKILL.md
sed -n '1,180p' plugins/ci/skills/payload-autodl-json/scripts/validate.py
echo "=== relevant tests and fixtures ==="
sed -n '1,240p' plugins/ci/skills/payload-results-yaml/scripts/test_validate.py
sed -n '1,100p' plugins/ci/skills/payload-results-yaml/scripts/testdata/invalid_rhcos_suspects.yaml
sed -n '1,100p' plugins/ci/skills/payload-results-yaml/scripts/testdata/valid_with_rhcos_suspects.yaml
echo "=== exact changelog_evidence references ==="
rg -n -C 3 'changelog_evidence|safe_dump|yaml\.dump|verbatim changelog|RPM Changelog Evidence' plugins/ci/skills/payload-analysis plugins/ci/skills/payload-results-yaml plugins/ci/skills/payload-autodl-jsonRepository: openshift-eng/ai-helpers
Length of output: 21815
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== diff metadata ==="
git diff --name-status
git diff --stat
echo "=== RHCOS schema field tables ==="
awk 'NR>=179 && NR<=194 {print NR ":" $0}' plugins/ci/skills/payload-results-yaml/SKILL.md
awk 'NR>=35 && NR<=115 {print NR ":" $0}' plugins/ci/skills/payload-autodl-json/SKILL.md
echo "=== autodl RHCOS operations and examples ==="
rg -n -C 6 'rhcos_change_suspected|rhcos_suspect_packages|RHCOS|changelog|Update|row' plugins/ci/skills/payload-autodl-json/SKILL.md plugins/ci/skills/payload-autodl-json/scripts/validate.py
echo "=== YAML validator test assertions ==="
rg -n -C 5 'rhcos|missing|type|assert|validate' plugins/ci/skills/payload-results-yaml/scripts/test_validate.py
echo "=== source-level verifier ==="
python3 - <<'PY'
import ast
from pathlib import Path
validator = Path("plugins/ci/skills/payload-results-yaml/scripts/validate.py")
tree = ast.parse(validator.read_text())
constants = {}
for node in tree.body:
if isinstance(node, ast.Assign) and len(node.targets) == 1 and isinstance(node.targets[0], ast.Name):
try:
constants[node.targets[0].id] = ast.literal_eval(node.value)
except Exception:
pass
print("REQUIRED_RHCOS_SUSPECT_FIELDS =", constants.get("REQUIRED_RHCOS_SUSPECT_FIELDS"))
func = next(n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name == "validate")
checks = []
for node in ast.walk(func):
if isinstance(node, ast.For) and isinstance(node.target, ast.Name) and node.target.id == "field":
checks.append(ast.unparse(node))
print("field-loop checks:")
for check in checks:
print(check)
sample = {
"metadata": {"payload_tag": "p", "version": "v", "stream": "s", "architecture": "a"},
"failing_jobs": [],
"candidates": [],
"rhcos_suspects": [{"rhcos_tag": "rhel-coreos-10", "package": "systemd", "failing_jobs": []}],
}
required = constants["REQUIRED_RHCOS_SUSPECT_FIELDS"]
missing = [f for f in required if f not in sample["rhcos_suspects"][0]]
print("sample missing required fields =", missing)
print("sample omits changelog_evidence =", "changelog_evidence" not in sample["rhcos_suspects"][0])
PY
echo "=== standalone YAML scalar behavior probe ==="
python3 - <<'PY'
try:
import yaml
except Exception as exc:
print("PyYAML unavailable:", exc)
raise SystemExit(0)
value = "first line\nsecond: line\n- literal"
encoded = yaml.safe_dump({"changelog_evidence": value}, sort_keys=False)
print(encoded, end="")
print("round_trip_equal =", yaml.safe_load(encoded)["changelog_evidence"] == value)
PYRepository: openshift-eng/ai-helpers
Length of output: 29968
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== payload-analysis contract around subagent output and Step 8 ==="
sed -n '300,330p' plugins/ci/skills/payload-analysis/SKILL.md
sed -n '680,735p' plugins/ci/skills/payload-analysis/SKILL.md
echo "=== all repository contract references ==="
rg -n -C 2 'rhcos_suspects|rhcos_suspect_packages|changelog_evidence|RPM Changelog Evidence' \
plugins/ci/skills --glob '*.md' --glob '*.py' --glob '*.json' --glob '*.yaml' | \
rg 'payload-analysis|payload-results-yaml|payload-autodl-json|rhcos_suspects|rhcos_suspect_packages|changelog_evidence' | head -300
echo "=== open PR inventory: filter relevant titles ==="
python3 - <<'PY'
import json
import urllib.request
import urllib.parse
base = "https://api.github.com/repos/openshift-eng/ai-helpers/pulls"
params = urllib.parse.urlencode({"state": "open", "per_page": 100})
req = urllib.request.Request(
f"{base}?{params}",
headers={"Accept": "application/vnd.github+json", "User-Agent": "review-contract-check"},
)
try:
with urllib.request.urlopen(req, timeout=20) as response:
pulls = json.load(response)
except Exception as exc:
print("OPEN_PR_QUERY_ERROR", type(exc).__name__, str(exc))
raise SystemExit(0)
terms = ("payload", "rhcos", "rpm", "changelog", "autodl", "results yaml", "triage")
print("open_pr_count =", len(pulls))
for pr in pulls:
title = pr.get("title", "")
if any(term in title.lower() for term in terms):
print(pr["number"], title, pr["html_url"], pr["head"]["ref"])
PYRepository: openshift-eng/ai-helpers
Length of output: 17449
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== open PR inventory ==="
curl -skL -H 'Accept: application/vnd.github+json' \
'https://api.github.com/repos/openshift-eng/ai-helpers/pulls?state=open&per_page=100' |
python3 -c '
import json, sys
data=json.load(sys.stdin)
print("open_pr_count =", len(data))
terms=("payload","rhcos","rpm","changelog","autodl","results yaml","triage")
for pr in data:
title=pr.get("title","")
if any(t in title.lower() for t in terms):
print(pr["number"], title, pr["html_url"])
'
echo "=== existing overlapping contracts ==="
python3 - <<'PY'
from pathlib import Path
paths = [
Path("plugins/ci/skills/payload-analysis/SKILL.md"),
Path("plugins/ci/skills/payload-results-yaml/SKILL.md"),
Path("plugins/ci/skills/payload-autodl-json/SKILL.md"),
Path("plugins/ci/skills/payload-results-yaml/scripts/validate.py"),
Path("plugins/ci/skills/payload-autodl-json/scripts/validate.py"),
]
terms = ("RHCOS", "rhcos_suspects", "changelog_evidence", "autodl", "payload-results")
for path in paths:
text = path.read_text()
hits = sum(text.lower().count(term.lower()) for term in terms)
print(f"{path}: contract_term_hits={hits}")
PYRepository: openshift-eng/ai-helpers
Length of output: 332
🏁 Script executed:
#!/bin/bash
set -eu
echo "=== GitHub API response diagnostics ==="
curl -skL -D /tmp/ai_helpers_headers \
-H 'Accept: application/vnd.github+json' \
'https://api.github.com/repos/openshift-eng/ai-helpers/pulls?state=open&per_page=100' \
-o /tmp/ai_helpers_pulls.json
head -20 /tmp/ai_helpers_headers
python3 - <<'PY'
import json
from pathlib import Path
data=json.loads(Path("/tmp/ai_helpers_pulls.json").read_text())
print("response_type =", type(data).__name__)
if isinstance(data, dict):
print(json.dumps(data, indent=2)[:2000])
else:
print("open_pr_count =", len(data))
terms=("payload","rhcos","rpm","changelog","autodl","results yaml","triage")
for pr in data:
title=pr.get("title","")
if any(t in title.lower() for t in terms):
print(pr["number"], title, pr["html_url"])
PY
echo "=== existing overlapping contracts ==="
python3 - <<'PY'
from pathlib import Path
paths = [
Path("plugins/ci/skills/payload-analysis/SKILL.md"),
Path("plugins/ci/skills/payload-results-yaml/SKILL.md"),
Path("plugins/ci/skills/payload-autodl-json/SKILL.md"),
Path("plugins/ci/skills/payload-results-yaml/scripts/validate.py"),
Path("plugins/ci/skills/payload-autodl-json/scripts/validate.py"),
]
terms = ("RHCOS", "rhcos_suspects", "changelog_evidence", "autodl", "payload-results")
for path in paths:
text = path.read_text()
hits = sum(text.lower().count(term.lower()) for term in terms)
print(f"{path}: contract_term_hits={hits}")
PYRepository: openshift-eng/ai-helpers
Length of output: 1892
Align the RHCOS suspect schema across outputs.
payload-analysis requires changelog_evidence, but payload-results-yaml/SKILL.md and validate.py omit it. Add the field, validate its string type and agreed sentinels ("none" and "unavailable"), add regression tests, and preserve multiline evidence with safe YAML serialization. Decide whether payload-autodl-json must retain this evidence; its contract currently stores only the suspicion flag and package names.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 76: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 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/ci/skills/payload-analysis/SKILL.md` around lines 452 - 465, Align
the RHCOS suspect schema by adding changelog_evidence to
payload-results-yaml/SKILL.md and validate.py, enforcing string values with the
allowed sentinels "none" and "unavailable". Add regression coverage for
validation and preserve multiline evidence through safe YAML serialization.
Review payload-autodl-json’s existing contract and retain its current fields
unless the schema requires this evidence.
|
|
||
| <!-- When changelog evidence exists for any suspect --> | ||
| <details> | ||
| <summary>RPM Changelog Evidence</summary> | ||
| <div class="changelog-evidence"> | ||
| <h4>{package} ({old_version} → {new_version})</h4> | ||
| <pre>{verbatim changelog entries from the RPM changelog diff}</pre> | ||
| <!-- Repeat for each suspect with changelog_evidence != "none" --> | ||
| </div> | ||
| </details> |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
HTML-escape changelog evidence before insertion.
The template places {verbatim changelog entries} inside HTML. Treat changelog text as untrusted input. Escape it before interpolation. Otherwise a crafted entry can break the report markup or execute script when a user opens the report.
Proposed fix
- <pre>{verbatim changelog entries from the RPM changelog diff}</pre>
+ <pre>{html_escape(changelog_evidence)}</pre>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <!-- When changelog evidence exists for any suspect --> | |
| <details> | |
| <summary>RPM Changelog Evidence</summary> | |
| <div class="changelog-evidence"> | |
| <h4>{package} ({old_version} → {new_version})</h4> | |
| <pre>{verbatim changelog entries from the RPM changelog diff}</pre> | |
| <!-- Repeat for each suspect with changelog_evidence != "none" --> | |
| </div> | |
| </details> | |
| <!-- When changelog evidence exists for any suspect --> | |
| <details> | |
| <summary>RPM Changelog Evidence</summary> | |
| <div class="changelog-evidence"> | |
| <h4>{package} ({old_version} → {new_version})</h4> | |
| <pre>{html_escape(changelog_evidence)}</pre> | |
| <!-- Repeat for each suspect with changelog_evidence != "none" --> | |
| </div> | |
| </details> |
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 76: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 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/ci/skills/payload-analysis/SKILL.md` around lines 632 - 641,
HTML-escape each changelog entry before interpolating it into the
`changelog-evidence` `<pre>` block in the RPM Changelog Evidence template.
Preserve the displayed changelog text while ensuring crafted entries cannot
alter report markup or execute script.
|
/test eval-payload-analysis |
The payload snapshot now includes RPM changelog diffs (what actually changed in each RPM bump, not just version numbers). Update the payload-analysis skill to consume this data: document the data structures (Step 3.7b), instruct subagents to read changelogs when investigating RHCOS RPM correlations (Step 4), use changelog text to sharpen correlation confidence in Step 6.1b, surface changelog evidence in the HTML report (Step 7.3b), and check for unused changelog evidence in the completeness review (Step 9). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
760376b to
34bcd6e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@plugins/ci/skills/payload-analysis/SKILL.md`:
- Around line 319-320: Update the payload schema around
rhcos_rpm_changelog_evidence to use a package-keyed map or list instead of one
unstructured value, preserving explicit "none" and "unavailable" states per
package. Ensure each rhcos_suspects[] record carries its corresponding
changelog_evidence so evidence remains mapped to the suspect package.
- Line 261: Revise the “Subpackage deduplication” guidance so identical version
bumps and changelog text are not treated as proof of a shared SRPM. When
source-RPM identity is unavailable in diff.changed[], retain separate package
suspects or explicitly mark deduplication as provisional, ensuring unrelated
packages remain represented in rhcos_suspects[].
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 22994b68-ac06-4722-a261-d9c10a8c045c
📒 Files selected for processing (1)
plugins/ci/skills/payload-analysis/SKILL.md
|
|
||
| 2. **Per-hop report files** — `rpm_changelogs[]` entries also have a `changelogs` path pointing at `<target-tag>/rpm-changelogs/<variant>/<older-tag>.md`. Read these to find which intermediate payload introduced a specific package bump (useful when the originating payload for a failure is not the baseline). `payloads[]` entries for the target payload and for payloads whose RPMDBs could not be extracted have no `rpm_changelogs[]` at all — the field is absent, not empty. An intermediate hop where the RHCOS image did not change shows `changed: 0, added: 0, removed: 0`; use this to pinpoint which hop introduced a given RHCOS bump. | ||
|
|
||
| **Subpackage deduplication.** Multiple binary RPMs are often built from the same source RPM (SRPM) and share identical changelogs. When `diff.changed[]` contains several packages with the same version bump and the same changelog text, they come from one SRPM — read the changelog once and treat them as a single logical change, not separate suspects. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 4 \
-e 'srpm|source_rpm|source package' \
-e 'rpm_changelogs|diff\.changed|changelog' \
plugins/ci/skills/payload-snapshot \
plugins/ci/skills/payload-analysis \
--glob '*.md' --glob '*.py'Repository: openshift-eng/ai-helpers
Length of output: 50383
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files 'plugins/ci/skills/payload-analysis/SKILL.md' \
'plugins/ci/skills/payload-snapshot/*' \
'plugins/ci/skills/payload-snapshot/scripts/*' | sed -n '1,120p'
printf '%s\n' '--- relevant source symbols ---'
rg -n -C 3 \
'diff\.changed|rhcos_suspects|changelog_evidence|source_rpm|srpm|subpackage|dedup' \
plugins/ci/skills/payload-analysis/SKILL.md \
plugins/ci/skills/payload-snapshot/scripts \
--glob '*.py' --glob '*.md' | sed -n '1,260p'Repository: openshift-eng/ai-helpers
Length of output: 13085
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- RPM diff implementation ---'
rg -n -C 8 \
'def .*rpm|changed.*changelog|package.*old|package.*new|rpm_changelogs|diff =' \
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py | sed -n '1,360p'
printf '%s\n' '--- RPM diff tests and fixtures ---'
rg -n -C 8 \
'changed|changelog|source.?rpm|srpm|subpackage|same version|dedup' \
plugins/ci/skills/payload-snapshot/scripts/test_rpm_changelogs.py \
plugins/ci/skills/payload-snapshot/scripts/test_collection_completeness.py | sed -n '1,360p'
printf '%s\n' '--- RPM metadata fields in repository ---'
rg -n \
'sourcerpm|source_rpm|sourceRpm|SRPM|rpm.*source|source.*rpm' \
plugins/ci/skills/payload-snapshot plugins/ci/skills/payload-analysis \
--glob '*.py' --glob '*.md' | sed -n '1,240p'Repository: openshift-eng/ai-helpers
Length of output: 50380
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- exact RPM diff construction ---'
sed -n '1400,1615p' plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
printf '%s\n' '--- exact RPM query helpers ---'
rg -n '^def _rpm_versions|^def _rpm_changelogs|^class RpmChangelogDiffer' \
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
sed -n '1160,1246p' plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
printf '%s\n' '--- all source-RPM references in tracked files ---'
git grep -n -i -E 'source.?rpm|sourcerpm|srpm|source package' -- \
'plugins/ci/skills/payload-analysis' \
'plugins/ci/skills/payload-snapshot' || trueRepository: openshift-eng/ai-helpers
Length of output: 14240
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '3528,3605p' plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.py
printf '%s\n' '--- RPM query formats used by the implementation ---'
rg -n -C 5 \
'rpm.*--queryformat|--qf|SOURCERPM|RPMTAG_SOURCERPM|rpm -qa|rpm -q' \
plugins/ci/skills/payload-snapshot/scripts/payload_snapshot.pyRepository: openshift-eng/ai-helpers
Length of output: 5080
Do not treat matching changelogs as proof of a shared SRPM.
The generated diff.changed[] records contain only package names, versions, and changelog text. If source-RPM identity is unavailable, retain separate package suspects or mark deduplication as provisional. Otherwise, unrelated packages can be merged and omitted from rhcos_suspects[].
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 76: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 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/ci/skills/payload-analysis/SKILL.md` at line 261, Revise the
“Subpackage deduplication” guidance so identical version bumps and changelog
text are not treated as proof of a shared SRPM. When source-RPM identity is
unavailable in diff.changed[], retain separate package suspects or explicitly
mark deduplication as provisional, ensuring unrelated packages remain
represented in rhcos_suspects[].
| - rhcos_rpm_changelog_evidence: <for each suspect package, the specific changelog entry that relates to the failure, or "none" if the changelog was read but contained no relevant entries, or "unavailable" if no changelog data exists> | ||
| ``` |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
rg -n -C 8 \
'rhcos_rpm_changelog_evidence|rhcos_rpm_suspect_packages|rhcos_suspects' \
plugins \
--glob '*.md' --glob '*.py' --glob '*.json'Repository: openshift-eng/ai-helpers
Length of output: 17482
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- payload-analysis relevant sections ---'
sed -n '235,335p;440,475p;620,650p;775,800p' plugins/ci/skills/payload-analysis/SKILL.md
printf '%s\n' '--- payload-results-yaml RHCOS schema ---'
sed -n '78,100p;179,194p;195,205p' plugins/ci/skills/payload-results-yaml/SKILL.md
printf '%s\n' '--- all changelog-related references ---'
rg -n -C 5 'changelog|suspect package|RHCOS RPM' plugins/ci/skills --glob '*.md' --glob '*.py' --glob '*.yaml' --glob '*.json'Repository: openshift-eng/ai-helpers
Length of output: 50383
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
printf '%s\n' '--- ANALYSIS_RESULT consumers and definitions ---'
rg -n -C 4 'ANALYSIS_RESULT|rhcos_rpm_changelog_evidence|changelog_evidence' \
plugins/ci --glob '*.md' --glob '*.py' --glob '*.json' --glob '*.yaml'
printf '%s\n' '--- RHCOS suspect output requirements ---'
sed -n '452,466p;524,532p' plugins/ci/skills/payload-analysis/SKILL.md
printf '%s\n' '--- tracked files that could parse analysis results ---'
git ls-files plugins/ci | rg 'payload|analysis|yaml'Repository: openshift-eng/ai-helpers
Length of output: 24379
Make RHCOS changelog evidence package-addressable.
rhcos_rpm_suspect_packages is comma-separated, but rhcos_rpm_changelog_evidence is one unstructured value. Multiple suspects can therefore lose package-to-evidence mapping. Define a package-keyed map or list, and carry changelog_evidence into each rhcos_suspects[] record.
🧰 Tools
🪛 SkillSpector (2.5.1)
[warning] 76: [RA2] Session Persistence: Skill establishes unauthorized persistence across sessions via cron jobs, startup scripts, or state files. Session persistence allows an attacker to maintain access beyond the current interaction.
Remediation: Remove any persistence mechanisms (cron jobs, startup scripts, state files). Skills should not maintain state across sessions without explicit user consent.
(Rogue Agent (RA2))
🤖 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/ci/skills/payload-analysis/SKILL.md` around lines 319 - 320, Update
the payload schema around rhcos_rpm_changelog_evidence to use a package-keyed
map or list instead of one unstructured value, preserving explicit "none" and
"unavailable" states per package. Ensure each rhcos_suspects[] record carries
its corresponding changelog_evidence so evidence remains mapped to the suspect
package.
The payload snapshot now includes RPM changelog diffs (what actually changed in each RPM bump, not just version numbers). Update the payload-analysis skill to consume this data: document the data structures, instruct subagents to read changelogs when investigating RHCOS RPM correlations, use changelog text to sharpen correlation confidence, surface changelog evidence in the HTML report, and check for unused changelog evidence in the completeness review.
This is a weaker version of #688 by not containing changes supposed to unify PR and RPM change evaluation paths. I'd like to run some evals comparing the two changes.
Summary by CodeRabbit
New Features
Chores