New Skill for RHDH Test Plan Review by psrna · Pull Request #24 · redhat-developer/rhdh-skill

psrna · 2026-05-13T10:50:38Z

Reviews an RHDH test plan Jira ticket and suggests platform/integration version updates based on support lifecycle pages and RHDH release milestones. Use when given an RHDH test plan Jira ticket ID to check which platform/integration versions to add or remove. Use when asked to "update test plan", "review test plan", "check platform versions in test plan", "review RHDH test plan", "what platforms should we test for RHDH X", or "update supported versions in test plan".

zdrapela · 2026-05-13T11:48:45Z

+**URL:** https://learn.microsoft.com/en-us/azure/openshift/support-lifecycle  
+**What to extract:** Kubernetes version GA dates and end of support (EOL) dates.
+
+### AKS (Azure Kubernetes Service)


Hi, I just finished a set of skills for lifecycle management of K8s jobs. I placed them in openshift/release repo, because that's where the jobs are defined, but now I'm not convinced it's the best place, because they cannot be used in combination with yours skill.

You can check them for inspiration: openshift/release#79129. I chose a more programmatic approach, finding API endpoints and parsing JSONs.

My idea was mostly around keeping the Test Plan JIRA up to date for a given release and eventually having child tasks for suggested changes that are needed. As jira is the source of truth for our release execution, I wanted to have a skill that helps making sure we have valid coverage. But i see there is some overlap with yours. Is there any preference on what you would like to see should happen with the approaches?

Your approach is fine. In my skills, I used more API endpoints and JSON parsing in bash scripts, which is more token-efficient. Maybe I can extract the reusable skills for programmatically retrieving supported versions into this repository, so they can be reused by your skill.

zdrapela · 2026-05-13T11:53:36Z

+
+## Platforms
+
+### OCP (OpenShift Container Platform)


And I have similar for OCP: openshift/release#78203 😅

durandom

Review: RHDH Test Plan Review Skill

Great concept — automating the test plan version review against lifecycle pages is exactly the kind of tedious, error-prone work that should be a skill. The workflow is thorough (lifecycle sources, special rules per platform, interactive review, child task creation), and the fetch_schedule.py milestone parser is well-structured.

A few things need fixing before merge.

Blocking

1. Hardcoded macOS paths — breaks on Linux (see inline comments)

The Jira token path is hardcoded to /opt/homebrew/bin/.jira-token. The rhdh-jira skill discovers this dynamically — use the same pattern. Both Python scripts also have macOS-only gcloud fallback paths.

2. Not wired into orchestrator or README

Every other sub-skill is listed in skills/rhdh/SKILL.md (intake menu + routing table + skills_index) and README.md. Without this, the skill is undiscoverable. See PR #20 for an example of how to add the routing.

3. No tests for fetch_schedule.py

The script has several pure functions that are easy to test: normalize_version, parse_date, find_milestones, find_schedule_tab. Other scripts in this repo (analyze-pr.py, triage-prs.py) all have corresponding test files. The milestone-finding logic in particular (walking backwards from GA row, matching freeze keywords) is the kind of code that needs test coverage — it's parsing semi-structured sheet data with heuristics.

4. get_gcloud_token() is duplicated

Nearly identical function in both check_gsheets.py (line 22) and fetch_schedule.py (line 42), including the same hardcoded fallback paths. Extract to a shared module or have fetch_schedule.py call check_gsheets.py --json first and skip if it fails.

Non-blocking

5. uv run --script with Google API deps

This is a conscious exception to ADR-0002 (stdlib-only Python). The uv run --script pattern is reasonable here — the Google Sheets SDK is the right tool for authenticated API access, and uv is already used in this project for pytest. Worth noting in a comment or the PR description that this is intentional, not accidental.

6. SKILL.md structure doesn't follow the Agent Skills spec

Other skills use <essential_principles>, <intake>, <routing>, <reference_index> XML tags. The workflow is inlined directly into SKILL.md (339 lines) rather than living in a separate workflows/ file. Consider splitting to workflows/review-test-plan.md to match the project pattern and keep SKILL.md as the entry point.

7. Hardcoded SPREADSHEET_ID

If the sheet moves or is replaced, the script breaks silently. Make it a CLI argument with the current value as default.

8. @zdrapela's comment is worth acknowledging

They have overlapping lifecycle management work in openshift/release (PR #79129, PR #78203) — using API endpoints and JSON parsing for the same platforms. Could be a collaboration opportunity or at least worth cross-referencing in sources.md.

TL;DR: The domain expertise and workflow design are solid. Main asks: (1) fix portability — no hardcoded macOS paths, (2) wire into orchestrator + README, (3) add tests for the schedule parser, (4) deduplicate get_gcloud_token().

durandom · 2026-05-13T12:10:11Z

+
+```bash
+TOKEN_FILE="/opt/homebrew/bin/.jira-token"
+


Hardcoded macOS Homebrew path. The rhdh-jira skill discovers this dynamically:

ACLI_DIR=$(dirname "$(readlink -f "$(which acli)")") TOKEN_FILE="$ACLI_DIR/.jira-token"

See skills/rhdh-jira/references/auth.md for the pattern. Same issue in the Gotchas section (line 337).

This breaks on Linux and non-Homebrew macOS installs.

Hopefully resolved in recent commit. Thx!

durandom · 2026-05-13T12:10:11Z

+
+
+def get_gcloud_token():
+    gcloud = shutil.which("gcloud")


This function is nearly identical to fetch_schedule.py:get_gcloud_token() — same fallback paths, same logic. Deduplicate: either extract to a shared module, or have fetch_schedule.py call check_gsheets.py --json as a pre-flight check.

Also: the fallback paths (~/Downloads/google-cloud-sdk/, /opt/homebrew/bin/) are macOS-specific. On Linux, gcloud is typically at /usr/bin/gcloud, /usr/local/bin/gcloud, or /snap/bin/gcloud. Since shutil.which() is checked first and handles the common case, consider just dropping the hardcoded fallbacks entirely — if gcloud isn't in PATH, the user should fix their PATH.

Hopefully fixed in recent commit. thx

durandom · 2026-05-13T12:10:11Z

+from pathlib import Path
+
+SPREADSHEET_ID = "1knVzlMW0l0X4c7gkoiuaGql1zuFgEGwHHBsj-ygUTnc"
+


If this sheet is replaced or a new copy is made, the script breaks silently. Make it a CLI argument with this as the default:

parser.add_argument( "--sheet-id", default="1knVzlMW0l0X4c7gkoiuaGql1zuFgEGwHHBsj-ygUTnc", help="Google Sheets spreadsheet ID", )

The workflow in SKILL.md can still omit the flag (uses the default), but it becomes overridable.

Hopefully resolved in recent commit. Thx.

durandom · 2026-05-13T12:10:11Z

@@ -0,0 +1,248 @@
+#!/usr/bin/env -S uv run --script
+# /// script


This is a conscious exception to ADR-0002 (stdlib-only Python). The uv run --script pattern is a pragmatic choice — the Google Sheets SDK is the right tool here, and uv is already used in this project.

Worth adding a brief comment noting this is intentional:

# Note: uses uv run --script for Google API deps (see ADR-0002 for stdlib-only rationale; # Google Sheets SDK is an intentional exception)

hopefully fixed too.

durandom · 2026-05-13T12:10:11Z

+---
+
+# RHDH Test Plan Review
+


The workflow (Steps 1–8, ~300 lines) is inlined directly here. Other skills split this into SKILL.md (entry point with <intake>, <routing>, <reference_index>) and workflows/<name>.md (the actual process). This keeps SKILL.md scannable and the workflow separately readable.

Also missing the XML structure tags (<essential_principles>, <intake>, <routing>) that other skills use per the Agent Skills spec. See skills/rhdh-pr-review/SKILL.md or skills/overlay/SKILL.md for the pattern.

I have used the skill-maker skill. If this is a conceptual thing or a pattern we want to follow, would it be possible to reflect/embed those principles in the skill-maker skill?

Added routing & intake

Hopefully resolved too. Following project patterns.

Ruff F401; monkeypatch fixture does not require importing pytest. Co-authored-by: Cursor <cursoragent@cursor.com>

psrna · 2026-05-14T10:28:32Z

@durandom I believe that I've resolved all your feedback above. Please take a final look and possibly merge. Thank you!

gustavolira · 2026-05-14T12:45:51Z

@@ -0,0 +1,255 @@
+# Note: uses uv run --script for Google API deps (see ADR-0002 for stdlib-only rationale;
+# Google Sheets SDK is an intentional exception)
+#!/usr/bin/env -S uv run --script


The shebang needs to be the very first line of the file for the OS to recognize it when running ./fetch_schedule.py. As written, line 1 is a comment, so chmod +x && ./fetch_schedule.py would either fail or fall through to /bin/sh. Move the note comment below the shebang:

#!/usr/bin/env -S uv run --script # Note: uses uv run --script for Google API deps (see ADR-0002 for stdlib-only rationale; # Google Sheets SDK is an intentional exception)

gustavolira · 2026-05-14T12:45:51Z

+    for i, row in enumerate(rows):
+        cells = [str(c) for c in row]
+        row_text = " " + " ".join(cells).lower() + " "
+        version_match = ver in row_text or (version.lower().replace("rhdh", "").strip() in row_text)


This does plain substring matching -- ver in row_text. Searching for "1.1" would match a row containing "RHDH 1.10 GA" since "1.1" is a substring of "1.10". RHDH versions already cross this boundary. Should we use word-boundary regex instead?

version_match = bool(re.search(rf'\b{re.escape(ver)}\b', row_text))

The second branch of the OR (version.lower().replace("rhdh", "").strip()) always produces the same result as ver after normalize_version, so it can be dropped at the same time.

gustavolira · 2026-05-14T12:45:51Z

+- **Child task issuetype**: Use `"Task"` with a `parent` field. If that fails with 400, retry with `"issuetype": {"name": "Subtask"}`.
+- **Child task project key**: Use the same project key as the parent (e.g., `RHIDP`).
+
+</gotchas>


nit: Other workflows in the project (e.g., overlay workflows) include a <success_criteria> section, and test_skill_structure.py::TestWorkflowStructure validates this. The SKILL.md has success criteria, but the workflow file itself doesn't. Worth adding one here for consistency, even if the test currently only checks overlay workflows.

gustavolira · 2026-05-14T12:45:51Z

+            ["Code freeze", "2025-01-05"],
+            ["RHDH 2.0", "GA ", "2025-02-01"],
+        ]
+        assert fs.find_milestones(rows, "RHDH 2.0")["code_freeze"] == "2025-01-05"


nit: No test for the version substring collision -- searching for "1.1" when the sheet contains "1.10". Given RHDH versions have already crossed the 1.9/1.10 boundary, this would protect a fix:

def test_does_not_confuse_similar_versions(self): rows = [["RHDH 1.10", "GA date", "2025-08-01"]] assert fs.find_milestones(rows, "1.1") == {}

durandom · 2026-05-19T10:35:54Z

Re-review: blocking items addressed ✅

All four blocking items from my initial review are resolved — nice work, @psrna!

Two small things I noticed while re-checking. Non-blocking, up to you whether to fix now or in a follow-up:

1. `find_milestones` substring matching (from @gustavolira's review)

In fetch_schedule.py, ver in row_text means searching for version "1.1" would match rows containing "1.10". RHDH has already crossed the 1.9→1.10 boundary, so this could bite in practice. A word-boundary check (e.g. re.search(rf'\b{re.escape(ver)}\b', row_text)) would fix it.

2. `<skills_index>` table in orchestrator

The routing works (intake option 9 + routing table are both there), but the <skills_index> summary table in skills/rhdh/SKILL.md still only lists the original 4 skills — rhdh-test-plan-review is missing from that table.

durandom

lgtm, up to @psrna to either fix here, follow up, or leave as is

psrna force-pushed the test-plan-review branch 2 times, most recently from f29ddfc to d84ce0a Compare May 13, 2026 11:05

zdrapela reviewed May 13, 2026

View reviewed changes

durandom requested changes May 13, 2026

View reviewed changes

psrna added 6 commits May 13, 2026 17:21

New Skill for RHDH Test Plan Review

b5ce709

Fix linter errors

ebcf744

Fix hardcoded paths

0e5211f

Allow spreadsheet id be configured via arguments.

631bd50

Deduplicate get_gcloud_token()

09f6df8

Add routing

f5ae433

psrna force-pushed the test-plan-review branch from 0f59cd7 to f5ae433 Compare May 13, 2026 15:37

Add tests for fetch_schedule.py

929c3cb

psrna force-pushed the test-plan-review branch from 0f355c4 to 929c3cb Compare May 13, 2026 16:12

psrna and others added 2 commits May 13, 2026 18:14

fix(tests): drop unused pytest import in test_fetch_schedule

4677265

Ruff F401; monkeypatch fixture does not require importing pytest. Co-authored-by: Cursor <cursoragent@cursor.com>

Alignd to project pattern. Split workflows from skill.md.

6a0ebf6

psrna requested a review from durandom May 14, 2026 10:29

gustavolira reviewed May 14, 2026

View reviewed changes

Fix comment issue

a7b3e63

durandom approved these changes May 19, 2026

View reviewed changes

Fix skills_index, add rhdh-test-plan-review skill reference.

eed6015

durandom merged commit 7244040 into redhat-developer:main May 19, 2026
4 checks passed

		from pathlib import Path

		SPREADSHEET_ID = "1knVzlMW0l0X4c7gkoiuaGql1zuFgEGwHHBsj-ygUTnc"

		@@ -0,0 +1,248 @@
		#!/usr/bin/env -S uv run --script
		# /// script


		```bash
		TOKEN_FILE="/opt/homebrew/bin/.jira-token"

Uh oh!

Conversation

psrna commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durandom left a comment

Choose a reason for hiding this comment

Review: RHDH Test Plan Review Skill

Blocking

Non-blocking

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

psrna commented May 14, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

durandom commented May 19, 2026

Re-review: blocking items addressed ✅

1. find_milestones substring matching (from @gustavolira's review)

2. <skills_index> table in orchestrator

Uh oh!

durandom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

psrna commented May 13, 2026 •

edited

Loading

1. `find_milestones` substring matching (from @gustavolira's review)

2. `<skills_index>` table in orchestrator