feat: add raise-pr and bug-fix skills for Jira and github issues for rhdh-plugins and community-plugins - #54
Conversation
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
|
Raise PR Test : redhat-developer/rhdh-plugins#3708 (comment) |
durandom
left a comment
There was a problem hiding this comment.
Review
Ambitious PR adding two full skills (`raise-pr` + `bug-fix`) that automate the PR lifecycle and end-to-end bug fixing with Playwright recordings. The architecture is well-thought-out — `bug-fix` chains into `raise-pr` with a clean caller-context contract.
What's good
- Repo profiles (`repo-profiles.md`) — auto-detection via `git remote -v` with separate templates for rhdh-plugins vs community-plugins is practical
- Baseline diffing — capturing `git status --porcelain` before builds to filter out pre-existing dirty files in Step 7 is clever and prevents accidental staging of unrelated changes
- Video recording workflow — the principled approach (write repro test → fail → capture before → fix → pass → capture after) with hard gates at each step prevents out-of-order execution
- GitHub Contents API for GIF upload — embedding recordings directly in the PR description via branch-committed files that auto-delete on merge is clean
- Jira input parsing (`jira-input.md`) — handles all common input formats (bare key, browse URL, URL without scheme, URL with query params)
- Changeset generation rules — correctly scoping to `plugins/` only, ignoring `packages/`, and checking published paths is right
Issues
-
Cursor-specific references. Several places reference Cursor-specific constructs:
- `CallMcpTool: server="user-jira"` in Step 11 — not portable to Claude Code or other agents
- `required_permissions: ["all"]` in `preflight_system_limits` — Cursor Shell tool concept
- The Prerequisites mention "Cursor" specifically for MCP setup
Consider abstracting the MCP calls behind a generic pattern (e.g., "use your agent's Jira MCP tool to add a comment") and noting Cursor as one implementation option.
-
PR size. Two full skills (700+ lines of SKILL.md each) plus 3 reference files in one PR is a lot to review. If this hasn't been through internal testing yet, consider splitting `raise-pr` into its own PR — it's independently useful and simpler to validate. `bug-fix` can follow as a separate PR that chains into it.
-
`workspace-map.md` will go stale. The Jira Component → workspace mapping is hardcoded. Consider generating this from the workspace directories + some metadata, or at least noting a date/version.
-
`ulimit -n 65536` in principles. This is macOS-specific (Linux typically has higher defaults). The principle should note this is primarily for macOS development.
-
Version conflict. This bumps 0.6.1 → 0.7.0. PR #58 also bumps 0.6.1 → 0.7.0. If both need to be v0.7.0 (minor for new skills), only one can merge with that version.
Recommendation
The skills are well-designed. Address the Cursor-specific portability concerns and consider splitting into two PRs for easier review. Needs version coordination with #58.
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> # Conflicts: # .claude-plugin/marketplace.json # .claude-plugin/plugin.json # uv.lock
8201527 to
6b3a527
Compare
|
@durandom Thanks for the thorough review! Addressing each point: 1. Cursor-specific references — Fixed. Removed all Cursor-specific constructs ( 2. PR size — Keeping as one PR since Tested end-to-end on two real issues:
3. 4. 5. Version conflict — Currently keeping v0.7.0 — if #58 merges first, I'll bump to v0.8.0 before this one goes in. Since the review, also added:
Hence I have updated the PR description and architecture diagram. Cc: @christoph-jerolimov @Eswaraiahsapram @lokanandaprabhu @sanketpathak |
|
First bug fix PR redhat-developer/rhdh-plugins#3741 raised by using these both skill which was invoked using |
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com>
|
Another PR redhat-developer/rhdh-plugins#3846 successfully merged , which was fixed/raised using |
|
I have used
Your login is fine — the failure was on the agent side, not yours.
So we can probably improve the gh authentication verification of
|
|
@ciiay Thanks for trying it out and the valuable feedback! 1. 2. Adding PR URL to the 3. E2E tests passing locally but failing on CI — I've raised around 10 PRs across both |
|
@its-mitesh-kumar Thanks for the reply. Then let's manually add the Git Pull Request value on the ticket once the PR is out. This way it's easier for scrum master to get a summary for stories that already has PRs and grab their review status. |
Review: tried
|
|
@teknaS47 Thanks for trying it out and the detailed feedback! 1. Check for existing PRs — Interesting idea, but adding PR deduplication would increase the scope significantly and scatter the skill's focus. The skill is designed to go from issue → fix → PR as a single flow. Checking and extending existing PRs is a different workflow. For now, keeping it out of scope. 2. Deeper verification for UI bugs — Good callout. The skill currently trusts a single Playwright assertion as proof of fix, which can be misleading for UI bugs (e.g., element exists in DOM but isn't visually correct). This is a valuable enhancement — will look into strengthening the verification step to go beyond just a green assertion. Tracked as a follow-up. Appreciate the real-world testing! |
Signed-off-by: its-mitesh-kumar <itsmiteshkumar98@gmail.com> # Conflicts: # pyproject.toml # uv.lock
|
Another PR by @ciiay using the skill : redhat-developer/rhdh-plugins#4021 |
christoph-jerolimov
left a comment
There was a problem hiding this comment.
Hi @its-mitesh-kumar, awesome work, its really cool. 👍
I like to merge this to make additional contributions easier. It would be nice if you can take a look into the comments below anyway so that the skill doesn't touch my (personal) computer as it does today.
Personally, I would prefix all skills in rhdh-skill with rhdh-, maybe you can check with @durandom (when he's back from PTO), @kadel and @schultzp2020 if they prefer this as well.
It would be also interesting to see how fullsend can benefit from this. Checkout with @durandom on this again.
| - **Jira access (if Jira issue)** — At least ONE of: | ||
| - `acli` on PATH + `.jira-token` configured. [ACLI docs](https://developer.atlassian.com/cloud/acli/guides/how-to-get-started/) | ||
| - Jira MCP (Atlassian Rovo MCP server). [Setup guide](https://support.atlassian.com/atlassian-rovo-mcp-server/docs/getting-started-with-the-atlassian-remote-mcp-server/) |
There was a problem hiding this comment.
It should load and use the rhdh-jira skill instead. As an improvement for later.
| - Assert the **expected** behavior (the assertion should fail when the bug is present). | ||
| 3. **Pre-flight: kill stale dev server** — before running the test, ensure the dev-server port (read from `playwright.config.ts` `webServer.url`) is free: | ||
| ``` | ||
| lsof -ti:<PORT> | xargs kill -9 2>/dev/null || true |
There was a problem hiding this comment.
Can we avoid this in the future please. I would be nice if the skill choise a random port, sets some Backstage or Playwright environment variable to start the server on a non 3000 port so that it doesn't interrupt my on going work:
Something like:
APP_CONFIG_appPort=
APP_CONFIG_backendPort=
(The exact variables might look different)
| ``` | ||
| 4. Run the test against the `en` locale in legacy mode. **Always** prefix with `ulimit -n 65536` and ensure the command runs without sandbox restrictions: | ||
| ``` | ||
| ulimit -n 65536 && APP_MODE=legacy npx playwright test e2e-tests/_repro-<KEY>.test.ts --project=en |
There was a problem hiding this comment.
Can you please don't configure my computer for me. From my understanding ulimit is a configuration that persist globally on the OS until I restart.
|
|
||
| 1. Re-run the reproduction test (with `ulimit` and without sandbox restrictions): | ||
| ``` | ||
| ulimit -n 65536 && APP_MODE=legacy npx playwright test e2e-tests/_repro-<KEY>.test.ts --project=en |
There was a problem hiding this comment.
Don't run ulimit all the time. It changes my computer afaik.
| # Jira Input Parsing & REST API Patterns | ||
|
|
||
| Shared reference for resolving Jira issue keys from user input and performing Jira REST API operations. Used by `raise-pr` (Step 1.5, Step 11) and `bug-fix` (Step 1). | ||
|
|
||
| ## Parsing Jira References | ||
|
|
||
| Accept any of these formats and normalize to a key + URL pair: | ||
|
|
||
| | Input format | Example | Extraction | | ||
| |-------------|---------|------------| | ||
| | Bare key | `RHDHBUGS-1934` | Match directly | | ||
| | Browse URL | `https://redhat.atlassian.net/browse/RHDHBUGS-1934` | Extract after `/browse/` | | ||
| | URL without scheme | `redhat.atlassian.net/browse/RHIDP-15252` | Extract after `/browse/` | | ||
| | URL with query params | `https://redhat.atlassian.net/browse/RHIDP-15252?focusedId=123` | Extract key between `/browse/` and `?` | | ||
|
|
||
| ### Extraction rules | ||
|
|
||
| 1. If the input contains `atlassian.net/browse/`, extract the path segment immediately after `/browse/` (strip any query string or fragment). | ||
| 2. Otherwise, scan the full input string for a match against the pattern `(RHIDP|RHDHBUGS|RHDHPLAN|RHDHSUPP)-\d+`. | ||
| 3. If neither matches, the input is invalid — ask the user to provide a valid key or URL. | ||
|
|
||
| ### Normalization | ||
|
|
There was a problem hiding this comment.
Instead of jira-input, couldn't we use rhdh-jira instead?
Summary
raise-prskill that automates the full PR workflow (detect workspace, build, generate changeset, commit, push, create PR) for bothrhdh-pluginsandcommunity-pluginsmonoreposgit remote -vand adapts PR template, npm scope, and upstream target accordingly--aauto-approve mode to skip all approval gates for unattended workflows#N) as bug referencesredhat-developer/rhdh-pluginsandbackstage/community-plugins, with repo-specific PR templates, commit conventions, and issue-linking rules## Fixedsection, post-PR comment, issue transition to Review, and web link via REST APIFixesin PR body for auto-close, post-PR comment, label managementbug-fixskill for end-to-end bug fixing with two modes: full-e2e (UI bugs with Playwright before/after recordings) and no-e2e (backend bugs with unit/integration test verification)not-ready-for-agentlabel and a missing-info comment if details are insufficient--no-verifymode forbug-fixto skip the user verification gate and auto-chain intoraise-pr --aArchitecture
Context
Extracts and generalizes the existing Cursor command (
rhdh-plugins/.cursor/commands/pr.md) and its #3679 enhancement into a portable agent skill that works across both plugin monorepos.The
raise-prskill has first-class issue support — any PR linked to a Jira issue gets automatic branch naming (fix/<workspace>-<JIRA-KEY>-<slug>), aFixes:commit trailer, a## Fixedsection in the PR body, post-PR Jira comment via MCP, issue transition to Review via ACLI, and web link via REST API. GitHub issues getFixes <url>in the PR body for auto-close, a post-PR comment, and label updates.The
bug-fixskill chains intoraise-prafter reproducing and fixing a bug. For UI bugs (full-e2e mode): writes a Playwright reproduction test with video recording, captures diagnostic context (screenshot + DOM + computed styles), applies the fix, captures before/after recordings, converts to GIFs, generates a test plan, and creates the PR with embedded visual evidence. For backend bugs (no-e2e mode): checks existing tests or writes a temporary repro unit test, diagnoses and fixes, verifies tests pass, deletes the temp test, and chains into raise-pr.Before any fix attempt, the skill validates the issue has sufficient detail (steps to reproduce, expected behavior, component/workspace). Issues missing required fields are automatically labeled
not-ready-for-agentwith a comment listing what's needed.How to Use
Prerequisites
ghCLI — GitHub CLI must be installed and authenticated (gh auth statusshould show logged in). Install: https://cli.github.com/rhdh-pluginsorcommunity-pluginsyarnavailable on PATHffmpegon PATH (for converting screen recordings to GIFs). Install if missing:which ffmpeg || brew install ffmpegInstall
Install from local checkout (while PR is unmerged):
After merge:
Invoke
What it does
not-ready-for-agentif missing infoworkspaces/lightspeed) and determines if the target isrhdh-pluginsorcommunity-pluginsraise-pr— builds, generates changeset, commits with DCO, pushes, uploads GIFs, creates PR with conditional sections, performs post-PR issue updates (Jira: comment + transition + web link; GitHub: comment + auto-close)Using raise-pr standalone
PR Body Sections (conditional)
issue_sourceis set (Jira for rhdh-plugins only; GitHub for both repos)test_planprovided by callerissue_sourceis present (bug-fix invoked raise-pr)Key Design Decisions
gh pr create— GIFs are uploaded to the fork branch via GitHub Contents API with DCO-compliant Signed-off-by trailers, and the resultingraw.githubusercontent.comURLs are embedded directly in the PR body.outerHTML, computed styles) before any fix is applied, providing concrete evidence for root-cause identification.Fixeskeyword.Files
raise-pr skill
skills/raise-pr/SKILL.md— 11-step workflow with repo auto-detection,--aauto-approve mode, dual-source issue resolution, GIF upload via Contents API, and post-PR issue updates (Jira comment/transition/web-link, GitHub comment/auto-close)skills/raise-pr/references/repo-profiles.md— per-repo config with conditional PR body templates, community-plugins Jira isolation rules, and DCO requirementsskills/raise-pr/references/jira-input.md— shared Jira key/URL parsing logic and REST API patterns (fetch, web link, comment, transition)bug-fix skill
skills/bug-fix/SKILL.md— multi-step workflow with dual-source input, readiness check, triage gate, workspace/repo detection, full-e2e and no-e2e modes,--no-verifyflagskills/bug-fix/references/workspace-map.md— Jira component and GitHub label to workspace mapping for both rhdh-plugins and community-plugins, with cross-cutting component disambiguationskills/bug-fix/references/github-input.md— GitHub issue URL parsing andghCLI patterns for fetching, commenting, and labelingskills/bug-fix/references/e2e-patterns.md— shared Playwright patterns across all workspacesskills/bug-fix/references/video-recording.md— Playwright video config, ffmpeg conversion, and automated GitHub Contents API embeddingShared
docs/agents/triage-labels.md— updated withnot-ready-for-agentlabel for agent-readiness triage gateMetadata
README.md— added Bug Fix section, updated raise-pr descriptionpyproject.toml,.claude-plugin/plugin.json,.claude-plugin/marketplace.json— version bump to 0.7.0Test plan
uv run pytestpasses (321 passed, 1 pre-existing failure unrelated to this PR)test_skill_structure.py)test_marketplace.py)npx skills add redhat-developer/rhdh-skill --skill raise-prand verify issue input parsing (Jira + GitHub)npx skills add redhat-developer/rhdh-skill --skill bug-fixand verify trigger phrasesraise-prwith a Jira key: verify branch name, commit trailer, PR body, and post-PR Jira comment/transition/web-linkraise-prwith a GitHub issue: verify PR bodyFixeslink, post-PR comment, auto-close behaviorbug-fixwith an RHDHBUGS issue: verify full workflow including triage gate, screenshot capture, test plan generation, GIF upload, and PR creationbug-fixtargeting community-plugins: verify no Jira info leaks into PR body or commit messagesbug-fix --no-verify: verify it auto-chains intoraise-pr --awithout pausing