allow links to additional data sources to further inform jira:generate-enhancement#409
Open
rvanderp3 wants to merge 1 commit intoopenshift-eng:mainfrom
Open
allow links to additional data sources to further inform jira:generate-enhancement#409rvanderp3 wants to merge 1 commit intoopenshift-eng:mainfrom
rvanderp3 wants to merge 1 commit intoopenshift-eng:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rvanderp3 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
Contributor
WalkthroughThe Jira plugin version is updated from 0.3.7 to 0.3.8 across manifest files. The generate-enhancement command documentation is extended to support an optional Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/jira/commands/generate-enhancement.md`:
- Around line 155-157: The fenced code blocks in
plugins/jira/commands/generate-enhancement.md showing the example invocations
(e.g., the lines containing "/jira:generate-enhancement OCPSTRAT-1596 ..." and
"/jira:generate-enhancement HIVE-2589 ...") are missing a language specifier and
trigger markdownlint MD040; update those triple-backtick fences (and the other
example fences noted around lines 161–163) to include a language tag such as
text or bash (e.g., ```text) so all example code fences consistently have a
language specifier.
In `@plugins/jira/skills/generate-enhancement/SKILL.md`:
- Around line 93-136: The process_additional_links function currently fetches
user-provided URLs directly; harden all calls to fetch_url_content and any
network fetches (e.g., fetch_github_pr, fetch_jira_issue) by validating URLs
first: enforce HTTPS only, check host allowlist (e.g., known
github/jira/docs/enhancements hosts), disallow localhost/127.0.0.0/8 and private
CIDR ranges, prevent file:// and other schemes, and reject or sanitize redirect
targets; also apply fetch limits (connection and read timeouts, max response
size) and follow-only-same-host redirect policy. Update process_additional_links
to run each link through a validate_and_normalize_url helper (create if needed)
before calling fetch_url_content/fetch_github_pr/fetch_jira_issue and ensure
those fetch functions respect timeouts, max bytes, and do not follow untrusted
redirects.
- Around line 648-649: The current risk-extraction condition checks
issue['status'] != 'Closed', which can treat resolved/done issues as active
risks; update the condition in the block that appends to risks (where issue and
risks are used) to use unresolved semantics or an explicit
closed-status/resolution set — e.g., check issue.get('resolution') is None or
that issue['status'] not in ['Closed','Done','Resolved','Cancelled'] (or similar
project-specific closed statuses) before calling risks.append({...}) so only
truly open/unresolved Bugs and Tasks are flagged as risks.
- Around line 140-142: The code calls match.groups() on the result of
re.match(r'https://github.com/([^/]+)/([^/]+)/pull/(\d+)', pr_url) without
checking for a successful match, which will raise on malformed URLs; update the
logic around re.match/pr_url to first check if match is not None (e.g., if not
match: raise ValueError or log and skip) before calling match.groups(), and
handle the error path cleanly so org, repo, pr_number are only assigned when a
valid match exists.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 88dfc441-1421-4715-94af-7c747bb60346
📒 Files selected for processing (5)
.claude-plugin/marketplace.jsondocs/data.jsonplugins/jira/.claude-plugin/plugin.jsonplugins/jira/commands/generate-enhancement.mdplugins/jira/skills/generate-enhancement/SKILL.md
Comment on lines
+155
to
+157
| ``` | ||
| /jira:generate-enhancement OCPSTRAT-1596 --additional-links "https://github.com/openshift/api/pull/1234,https://issues.redhat.com/browse/OCPBUGS-5678" | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add languages to new fenced code blocks to satisfy markdownlint.
The new example fences are missing a language specifier, which triggers MD040. Use text or bash consistently for these command examples.
Suggested diff
- ```
+ ```text
/jira:generate-enhancement OCPSTRAT-1596 --additional-links "https://github.com/openshift/api/pull/1234,https://issues.redhat.com/browse/OCPBUGS-5678"
```
@@
- ```
+ ```text
/jira:generate-enhancement HIVE-2589 --additional-links "https://github.com/openshift/api/pull/1234,https://issues.redhat.com/browse/OCPBUGS-5678,https://github.com/openshift/enhancements/blob/master/enhancements/cluster-api/cluster-api-integration.md"
```Also applies to: 161-163
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 155-155: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/commands/generate-enhancement.md` around lines 155 - 157, The
fenced code blocks in plugins/jira/commands/generate-enhancement.md showing the
example invocations (e.g., the lines containing "/jira:generate-enhancement
OCPSTRAT-1596 ..." and "/jira:generate-enhancement HIVE-2589 ...") are missing a
language specifier and trigger markdownlint MD040; update those triple-backtick
fences (and the other example fences noted around lines 161–163) to include a
language tag such as text or bash (e.g., ```text) so all example code fences
consistently have a language specifier.
Comment on lines
+93
to
+136
| When `--additional-links` is provided, fetch and analyze additional context before generating the enhancement: | ||
|
|
||
| ```python | ||
| def process_additional_links(links): | ||
| """Process additional context links provided by the user.""" | ||
|
|
||
| context = { | ||
| 'github_prs': [], | ||
| 'jira_issues': [], | ||
| 'enhancements': [], | ||
| 'documentation': [] | ||
| } | ||
|
|
||
| for link in links.split(','): | ||
| link = link.strip() | ||
|
|
||
| # GitHub PR | ||
| if 'github.com' in link and '/pull/' in link: | ||
| pr_info = fetch_github_pr(link) | ||
| context['github_prs'].append(pr_info) | ||
|
|
||
| # Jira issue | ||
| elif 'issues.redhat.com' in link or 'jira.' in link: | ||
| issue_key = extract_jira_key(link) | ||
| issue_info = fetch_jira_issue(issue_key) | ||
| context['jira_issues'].append(issue_info) | ||
|
|
||
| # Enhancement proposal | ||
| elif 'enhancements' in link and link.endswith('.md'): | ||
| enhancement_content = fetch_url_content(link) | ||
| context['enhancements'].append({ | ||
| 'url': link, | ||
| 'content': enhancement_content | ||
| }) | ||
|
|
||
| # Documentation | ||
| elif 'docs.openshift.com' in link or '.md' in link: | ||
| doc_content = fetch_url_content(link) | ||
| context['documentation'].append({ | ||
| 'url': link, | ||
| 'content': doc_content | ||
| }) | ||
|
|
||
| return context |
Contributor
There was a problem hiding this comment.
Harden external link fetching to prevent SSRF/path abuse.
This guidance currently fetches user-provided URLs directly. Please document strict URL validation (HTTPS only, allowlisted hosts, no localhost/private CIDR, redirects/timeouts/size caps) before any fetch_url_content call.
Suggested diff
def process_additional_links(links):
"""Process additional context links provided by the user."""
@@
for link in links.split(','):
link = link.strip()
+ if not is_allowed_context_url(link):
+ continue
@@
- elif 'docs.openshift.com' in link or '.md' in link:
+ elif 'docs.openshift.com' in link or '.md' in link:
doc_content = fetch_url_content(link)
context['documentation'].append({
'url': link,
'content': doc_content
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/generate-enhancement/SKILL.md` around lines 93 - 136, The
process_additional_links function currently fetches user-provided URLs directly;
harden all calls to fetch_url_content and any network fetches (e.g.,
fetch_github_pr, fetch_jira_issue) by validating URLs first: enforce HTTPS only,
check host allowlist (e.g., known github/jira/docs/enhancements hosts), disallow
localhost/127.0.0.0/8 and private CIDR ranges, prevent file:// and other
schemes, and reject or sanitize redirect targets; also apply fetch limits
(connection and read timeouts, max response size) and follow-only-same-host
redirect policy. Update process_additional_links to run each link through a
validate_and_normalize_url helper (create if needed) before calling
fetch_url_content/fetch_github_pr/fetch_jira_issue and ensure those fetch
functions respect timeouts, max bytes, and do not follow untrusted redirects.
Comment on lines
+140
to
+142
| match = re.match(r'https://github.com/([^/]+)/([^/]+)/pull/(\d+)', pr_url) | ||
| org, repo, pr_number = match.groups() | ||
|
|
Contributor
There was a problem hiding this comment.
Guard regex parsing before dereferencing GitHub PR URL groups.
match.groups() will fail on malformed/non-PR URLs. Add a null check and skip/raise a controlled error.
Suggested diff
def fetch_github_pr(pr_url):
"""Fetch PR details from GitHub."""
match = re.match(r'https://github.com/([^/]+)/([^/]+)/pull/(\d+)', pr_url)
+ if not match:
+ return None
org, repo, pr_number = match.groups()📝 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.
Suggested change
| match = re.match(r'https://github.com/([^/]+)/([^/]+)/pull/(\d+)', pr_url) | |
| org, repo, pr_number = match.groups() | |
| def fetch_github_pr(pr_url): | |
| """Fetch PR details from GitHub.""" | |
| match = re.match(r'https://github.com/([^/]+)/([^/]+)/pull/(\d+)', pr_url) | |
| if not match: | |
| return None | |
| org, repo, pr_number = match.groups() |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/generate-enhancement/SKILL.md` around lines 140 - 142,
The code calls match.groups() on the result of
re.match(r'https://github.com/([^/]+)/([^/]+)/pull/(\d+)', pr_url) without
checking for a successful match, which will raise on malformed URLs; update the
logic around re.match/pr_url to first check if match is not None (e.g., if not
match: raise ValueError or log and skip) before calling match.groups(), and
handle the error path cleanly so org, repo, pr_number are only assigned when a
valid match exists.
Comment on lines
+648
to
+649
| if issue['type'] in ['Bug', 'Task'] and issue['status'] != 'Closed': | ||
| risks.append({ |
Contributor
There was a problem hiding this comment.
Use broader terminal-status filtering for Jira risk extraction.
Filtering only status != 'Closed' can include resolved/done bugs as active risks. Prefer unresolved semantics (or an explicit closed-status set).
Suggested diff
- if issue['type'] in ['Bug', 'Task'] and issue['status'] != 'Closed':
+ terminal_statuses = {'Closed', 'Done', 'Resolved', 'Cancelled'}
+ if issue['type'] in ['Bug', 'Task'] and issue['status'] not in terminal_statuses:
risks.append({
'source': issue['key'],
'description': issue['summary'],
'details': issue['description']
})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/jira/skills/generate-enhancement/SKILL.md` around lines 648 - 649,
The current risk-extraction condition checks issue['status'] != 'Closed', which
can treat resolved/done issues as active risks; update the condition in the
block that appends to risks (where issue and risks are used) to use unresolved
semantics or an explicit closed-status/resolution set — e.g., check
issue.get('resolution') is None or that issue['status'] not in
['Closed','Done','Resolved','Cancelled'] (or similar project-specific closed
statuses) before calling risks.append({...}) so only truly open/unresolved Bugs
and Tasks are flagged as risks.
|
PR needs rebase. DetailsInstructions 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 kubernetes-sigs/prow repository. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add
--additional-linksparameter to/jira:generate-enhancementcommand, enabling users to provide additional context (GitHub PRs, Jira issues, enhancement proposals, documentation)beyond the epic or feature to enrich the generated enhancement proposal.
Motivation
When generating enhancement proposals from Jira epics/features, users often have additional context that would improve the quality of the generated document:
Previously, users had to manually add this information after generation. This enhancement automates the enrichment process by fetching and analyzing additional links upfront.
Changes
Command Updates (
plugins/jira/commands/generate-enhancement.md)--additional-linksparameter accepting comma-separated URLsSkill Updates (
plugins/jira/skills/generate-enhancement/SKILL.md)Added Step 0: Process Additional Context Links with logic to:
Enhanced API Extensions section to:
Enhanced Risks and Mitigations section to:
Updated metadata generation to:
see-alsofrom related enhancement proposalsAdded content mapping reference table showing how each link type enriches specific sections.
Updated reporting to show which additional context was used and which sections were auto-populated.
Version Bump
0.3.7to0.3.8Usage Examples
Basic usage with API PR
Summary by CodeRabbit
New Features
--additional-linksparameter to thegenerate-enhancementcommand, enabling users to provide GitHub PRs, Jira issues, and documentation links to automatically enrich enhancement generation with API changes, risks, and related references.Documentation
Version: Bumped to 0.3.8