feat(rhdh-pr-review): add rhdh-chart PR review support

[Fortune-Ndlovu]

Summary

• Extends the rhdh-pr-review skill to support reviewing rhdh-chart PRs alongside the existing rhdh-operator PR support
• Extracts shared review phases into a reusable framework so adding future repos (rhdh main, rhdh-plugins, rhdh-cli) requires only ~200 lines of repo-specific content instead of duplicating the full ~500-line workflow

What changed

New shared framework (references/shared-*.md)

Phases that are structurally identical across repos are extracted into three shared references:

Reference	Phase	What it provides
shared-pr-fetch.md	Phase 1	PR context fetching, parameterized by REPO/PR_NUMBER
shared-verification.md	Phase 6	Active verification process (6.1 analyze diff → 6.2 propose plan + STOP gate → 6.3 execute with evidence)
shared-findings-structure.md	Phase 7	Findings report structure (7.1 summary table, 7.2-7.3 best-practice/security with caller-provided bullets, 7.4 improvements, 7.5 rollback)

Each repo-specific workflow references these via the existing pattern: "Read ../references/shared-X.md and follow the <section> instructions."

New chart support

• references/chart-pr-testing.md — Thin reference for chart CI behavior (no CI images, local checkout, helm template, ct lint)
• workflows/review-chart-pr.md — 7-phase chart review workflow (checkout PR branch → local validation → ensure cluster → helm upgrade from PR branch → diff-based checklist → active verification → findings & rollback)

What stays repo-specific

Phases 2-5 are fundamentally different per repo and remain inline:

Phase	Operator	Chart
2: Get artifacts	Extract CI-built images from PR comments	Checkout PR branch, helm dep update
3: Ensure cluster	Check for operator deployment + Backstage CR	Check for Helm release + RHDH pods
4: Deploy	OLM bundle or install.yaml manifests	helm upgrade from local checkout
5: Checklist	CRD/API, controller, default-config categories	Templates, values, schema, Chart.yaml categories

Best-practice bullets and rollback commands are also repo-specific, defined inline in each workflow.

Adding a future repo

After this PR, adding a new repo requires:

1. One reference file (~60 lines) — CI behavior, artifact extraction
2. One workflow file (~200-250 lines) — Phases 2-5 inline, Phases 1/6/7 via shared references
3. One intake/routing entry in SKILL.md

No touching shared references or existing workflows.

Design decisions

• No CI images for chart PRs — unlike operator PRs, chart CI runs ct lint + ct install on KIND without publishing container images. The workflow checks out the PR branch and deploys from the local checkout.
• Full chart deployment — follows the deploy_full_bundle principle from PR feat: add rhdh-pr-review skill for testing operator PRs on live clusters #20: deploys the complete chart via helm upgrade from the PR branch, not just a values override.
• Thin references — chart-pr-testing.md teaches where to look and how to interpret, pointing to rhdh-repos.md and upstream READMEs per PR feat: add rhdh-pr-review skill for testing operator PRs on live clusters #20 review feedback.
• Hybrid extraction — only genuinely identical phases (1, 6, 7 structure) are shared. Phases 2-5 differ too much to abstract without adding complexity. Envelope sections (tracking, success criteria) are small enough that extracting them adds indirection without saving space.

Test plan

• All 273 existing tests pass (pytest)
• All relative paths in reference index and workflows resolve correctly
• No hardcoded user-specific paths in new files
• Intake numbering and routing tables consistent across orchestrator and sub-skill
• /skill-maker audit against skills/rhdh-pr-review/SKILL.md
• Read each workflow top-to-bottom as an agent to confirm linear flow at reference boundaries

🤖 Generated with Claude Code

[durandom]

Good work on the shared/specific split — phases 1, 6, 7 are genuinely repo-agnostic and the extraction is well-structured. The chart workflow is thorough (covers both OpenShift and vanilla K8s, handles OLM conflict detection, has proper rollback).

1 blocking issue:

Non-OLM rollback commands dropped from operator workflow. In review-operator-pr.md, the refactored Phase 7.5 only includes OLM-managed rollback. The non-OLM path (oc apply -f /tmp/rollback-install.yaml) was lost during the extraction. Since the operator workflow supports both OLM (4.4a) and non-OLM (4.4b) deployment, both rollback paths must be preserved. Fix: add back the non-OLM rollback block to the operator-specific section of Phase 7.5.

5 non-blocking suggestions:

1. Operator security bullets lost specificity — the original mentioned environment variables and digest pinning explicitly; the shared version generalizes these. Consider keeping operator-specific supplements. Also the remaining operator bullet (go.mod CVEs) duplicates the generic version already in the shared file.
2. gh pr checkout --detach in chart-pr-testing.md assumes CWD is already a rhdh-chart clone — this isn't clear from context and will fail otherwise.
3. --reuse-values + --set in Helm upgrade (Phase 4.3) can surprise when the new chart introduces new required values or renames existing ones — worth noting --reset-values as a fallback.
4. find .. -maxdepth 1 -name "rhdh-chart" to locate the chart repo is fragile — depends on CWD being a sibling directory.
5. helm repo add bitnami + helm dependency update appears in both chart-pr-testing.md and review-chart-pr.md Phase 2.2 — pick one location to avoid maintenance drift.