chore(skills): Update duplicate-epic skill to work with bugs and stories#53
chore(skills): Update duplicate-epic skill to work with bugs and stories#53dlabrecq wants to merge 3 commits intopatternfly:mainfrom
Conversation
📝 WalkthroughWalkthroughAccepted any Jira issue as input, resolved it up parent links to an Epic (max depth), cloned the resolved Epic while removing Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/issue-management/skills/duplicate-epic/SKILL.md`:
- Around line 81-82: Update the "Clone" step in the SKILL.md "What the Script
Does" section to state that the epic description is not copied verbatim: the
script sanitizes the description by stripping mediaSingle and media nodes
(attachments/embedded media) before creating the PF clone, and note that
embedded attachments will be removed in the PF copy; keep the existing "Ensure
'is duplicated by' link" text unchanged but ensure the doc reflects this
behavior and its failure mode (attachments lost) so users aren't surprised.
- Line 3: The frontmatter "description" in SKILL.md is too long and will be
truncated in listings; shorten it to under 250 characters, front-load key use
cases and include the trigger context "/duplicate-epic <issue> <feature>"
(mentioning that it duplicates a Jira epic into the PatternFly project and links
back to the original), and remove excess detail so the core purpose and trigger
are visible in the skill listing.
🪄 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: 07d7fca9-c7eb-49f1-ad64-1889ef5ecea5
📒 Files selected for processing (2)
plugins/issue-management/skills/duplicate-epic/SKILL.mdplugins/issue-management/skills/duplicate-epic/scripts/duplicate_epic.sh
plugins/issue-management/skills/duplicate-epic/scripts/duplicate_epic.sh
Outdated
Show resolved
Hide resolved
| echo "Fetching $ORIGINAL_KEY..." | ||
| ORIGINAL_ISSUE=$(api_request GET "issue/$ORIGINAL_KEY") | ||
|
|
||
| ISSUE_TYPE=$(echo "$ORIGINAL_ISSUE" | jq -r '.fields.issuetype.name') |
There was a problem hiding this comment.
nit: The while loop walks up the hierarchy with no depth guard — if Jira data ever has a cycle (e.g., two issues listing each other as parent due to misconfiguration), would this loop run forever? A simple counter capping at ~10 iterations would be cheap insurance against a hang.
| @@ -129,16 +161,19 @@ else | |||
| issuetype: {name: "Epic"}, | |||
| assignee: {accountId: $account_id} | |||
| } | |||
There was a problem hiding this comment.
nit: This filter only removes top-level content nodes. If a mediaSingle node is nested inside a panel, expand, or layoutColumn, wouldn't it survive the filter? Would a recursive walk be more appropriate here, or are the epic descriptions you've seen always flat enough that top-level stripping is sufficient?
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/issue-management/skills/duplicate-epic/scripts/duplicate_epic.sh`:
- Around line 199-205: The call to api_request in the conditional can never
reach the elif/else because api_request itself calls exit on non-2xx responses;
run it in a subshell so its exit doesn't terminate the main script: replace the
direct call used in the if with a subshell invocation (e.g. run api_request POST
"issueLink" "$LINK_PAYLOAD" inside ( ) and redirect stdout/stderr as before) so
the conditional can test its exit status and allow the CREATED_NEW_CLONE /
NEW_KEY warning or abort branches to execute.
🪄 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: 193e08a6-c157-4ed8-8420-b1d1807a8e2f
📒 Files selected for processing (1)
plugins/issue-management/skills/duplicate-epic/scripts/duplicate_epic.sh
| if api_request POST "issueLink" "$LINK_PAYLOAD" >/dev/null 2>&1; then | ||
| echo "Link added" | ||
| elif [[ "$CREATED_NEW_CLONE" -eq 0 ]]; then | ||
| echo "WARNING: Could not add 'is duplicated by' link (likely missing link-issue permission in the source project). Continuing..." >&2 | ||
| else | ||
| echo "ERROR: Created $NEW_KEY but could not add the duplicate link; aborting to avoid future duplicate clones." >&2 | ||
| exit 1 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expected output: only "before" is printed.
# If that happens, the `elif`/`else` path is unreachable when the function uses `exit 1`.
set -euo pipefail
output=$(
bash -c '
api_request() { exit 1; }
echo before
if api_request; then
echo then
elif [[ 0 -eq 0 ]]; then
echo elif
else
echo else
fi
echo after
' 2>&1 || true
)
printf '%s\n' "$output"Repository: patternfly/ai-helpers
Length of output: 71
🏁 Script executed:
#!/bin/bash
# Find and examine the duplicate_epic.sh file
find . -name "duplicate_epic.sh" -type fRepository: patternfly/ai-helpers
Length of output: 139
🏁 Script executed:
#!/bin/bash
# Read the file to find the api_request function definition and the code at lines 199-205
cat -n ./plugins/issue-management/skills/duplicate-epic/scripts/duplicate_epic.sh | sed -n '80,90p; 195,210p'Repository: patternfly/ai-helpers
Length of output: 1280
🏁 Script executed (no clone):
#!/bin/bash
# Test if the proposed fix (subshell with parentheses) resolves the issue
# Expected output: both "before" and "elif" should be printed
set -euo pipefail
output=$(
bash -c '
api_request() { exit 1; }
echo before
if (api_request POST "issueLink" "$LINK_PAYLOAD" >/dev/null 2>&1); then
echo then
elif [[ 0 -eq 0 ]]; then
echo elif
else
echo else
fi
echo after
' 2>&1 || true
)
printf '%s\n' "$output"
Length of output: 48
The recoverable link-error path is unreachable.
api_request() calls exit 1 on line 84 for non-2xx responses. At line 199, invoking it directly without a subshell terminates the script before the elif / else branches can execute, so the warning path is never reached.
Wrap the call in a subshell to allow the conditional to handle the non-zero return:
Fix
- if api_request POST "issueLink" "$LINK_PAYLOAD" >/dev/null 2>&1; then
+ if (api_request POST "issueLink" "$LINK_PAYLOAD" >/dev/null 2>&1); then
echo "Link added"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/issue-management/skills/duplicate-epic/scripts/duplicate_epic.sh`
around lines 199 - 205, The call to api_request in the conditional can never
reach the elif/else because api_request itself calls exit on non-2xx responses;
run it in a subshell so its exit doesn't terminate the main script: replace the
direct call used in the if with a subshell invocation (e.g. run api_request POST
"issueLink" "$LINK_PAYLOAD" inside ( ) and redirect stdout/stderr as before) so
the conditional can test its exit status and allow the CREATED_NEW_CLONE /
NEW_KEY warning or abort branches to execute.
2 participants
Updated duplicate-epic skill to work with different JIra bugs in addition to stories.
SKILL.md
scripts/duplicate_epic.sh
Summary by CodeRabbit
New Features
Bug Fixes
Documentation