-
Notifications
You must be signed in to change notification settings - Fork 101
<CLEAN COMMIT HISTORY>: Add /jira:backlog command to find suitable tickets from backlog #130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
<CLEAN COMMIT HISTORY>: Add /jira:backlog command to find suitable tickets from backlog #130
Conversation
Regenerate PLUGINS.md and docs/data.json to include the new jira:backlog command in the plugin documentation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: ehearne-redhat The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Warning Rate limit exceeded@ehearne-redhat has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds a new Jira plugin command Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as /jira:backlog
participant Config as MCP Config (~/.config/claude-code/mcp.json)
participant JIRA as JIRA REST API
participant Processor as Batch Processor
participant Reporter as Report Generator
User->>CLI: invoke with [project-key] [--assignee] [--days-inactive]
CLI->>Config: read JIRA_PERSONAL_TOKEN / JIRA_API_TOKEN
Config-->>CLI: token (or error)
CLI->>CLI: validate args & build JQL
CLI->>JIRA: fetch issues (curl, paginated)
JIRA-->>CLI: issues pages
CLI->>Processor: save & process batches
Processor->>Processor: filter (unassigned/bot), group by priority
Processor->>Processor: select top 2 per priority
Processor-->>CLI: filtered.json, stats.json
CLI->>Reporter: render final report
Reporter-->>CLI: markdown/plain report
CLI-->>User: display report (and optional saved files)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (7 passed)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (4)
plugins/jira/commands/backlog.md (4)
46-55: Clarify the distinction between MCP configuration and direct API access.The prerequisites section introduces the MCP configuration requirement but then explains the command doesn't use MCP commands. This could confuse users about whether the MCP server actually needs to be running. Consider restructuring this section to:
- State upfront that credentials are read from the MCP configuration file (not that MCP commands are used)
- Clarify whether the MCP server itself needs to be running (or only the config file needs to exist)
- Simplify the "Why not use MCP commands?" section into a brief rationale
This would help users understand: "configure credentials here, but we fetch data via curl instead of through MCP."
360-408: Reconsider the definition of "bot-assigned" tickets in filtering logic.Line 307 shows the filtering logic checks:
if 'bot' in assignee_name.lower(). This simple string matching could be problematic:
- Names like "Robert" or "robot-admin" would incorrectly match as bots
- Different JIRA instances may use different naming conventions for bot accounts
- The approach is fragile and prone to false positives
Consider:
- Maintaining a configurable list of known bot account names/IDs
- Checking for a JIRA bot user flag/marker instead of string matching
- Documenting which bot accounts are recognized (OCP DocsBot, etc.)
384-384: Replace over-used "very" intensifiers for clarity.LanguageTool flagged two instances of "very" used as intensifiers:
- Line 384: "Zero comments AND very old (120+ days)"
- Line 529: "Can process very large backlogs (10,000+ tickets)"
These can be replaced with more precise language:
- "old (120+ days)" is already precise (the threshold does the work)
- "large backlogs (10,000+ tickets)" is already clear
Minor style improvement, not blocking.
Also applies to: 529-529
502-515: Example curl authentication command in error message exposes token.Line 511 shows an example error message that includes:
4. Test authentication: curl -H "Authorization: Bearer YOUR_TOKEN" YOUR_JIRA_URL/rest/api/2/myselfWhile
YOUR_TOKENis a placeholder, this example teaches users to pass tokens in curl headers. Consider:
- Add security note: Remind users never to paste actual tokens in curl commands or shell history
- Recommend safer alternatives: Suggest reading from environment variable instead (e.g.,
curl -H "Authorization: Bearer $JIRA_PERSONAL_TOKEN" ...)- Add history warning: Note that shell history may record the command with the real token if not careful
This is a best-practice security improvement, not a blocker.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (3)
PLUGINS.md(1 hunks)docs/data.json(1 hunks)plugins/jira/commands/backlog.md(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
plugins/jira/commands/backlog.md
[high] 511-511: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
plugins/jira/commands/backlog.md
[style] ~384-~384: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...lready complete) - Zero comments AND very old (120+ days) - likely abandoned - Com...
(EN_WEAK_ADJECTIVE)
[style] ~529-~529: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...limits - Parallel-safe: Can process very large backlogs (10,000+ tickets) without issu...
(EN_WEAK_ADJECTIVE)
🪛 markdownlint-cli2 (0.18.1)
plugins/jira/commands/backlog.md
10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
80-80: Bare URL used
(MD034, no-bare-urls)
81-81: Bare URL used
(MD034, no-bare-urls)
141-141: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
172-172: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
233-233: Code block style
Expected: fenced; Actual: indented
(MD046, code-block-style)
267-267: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
342-342: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
343-343: Unordered list indentation
Expected: 0; Actual: 3
(MD007, ul-indent)
413-413: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
483-483: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
493-493: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
504-504: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (7)
PLUGINS.md (1)
102-102: Command documentation properly integrated.The new
/jira:backlogcommand is correctly documented with appropriate signature and description. The placement in the Jira section follows the established pattern.docs/data.json (1)
82-87: Metadata properly structured and placed.The new backlog command object is correctly formatted JSON with all required fields (name, description, synopsis, argument_hint). The placement after create-release-note maintains logical ordering.
plugins/jira/commands/backlog.md (5)
85-110: Verify MCP server container setup is necessary for this command.The documentation recommends starting the MCP container (lines 85-110), but then states the command uses curl directly (not MCP commands). Please clarify:
- Does the
/jira:backlogcommand actually require the MCP server to be running, or only the config file?- If the MCP server isn't required, can users skip the "Start Local MCP Server with Podman" section?
- What is the actual purpose of starting the container if this command bypasses it?
This distinction is important for user experience—if the container isn't required, users shouldn't feel obligated to run it.
153-350: Step 6 (intelligent analysis) requires critical clarification on implementation scope.Lines 350-409 describe "intelligently analyzing and selecting best tickets"—this appears to require human judgment, not automation. The documentation states "DO NOT analyze inside Python/bash" (line 369) and asks the user to "read and analyze" (line 371), implying Claude (the user's assistant) must perform subjective analysis.
However, it's unclear whether:
- The implementation is meant to be automated within Claude Code (i.e., Claude performs the analysis), or
- This is a guide for manual human analysis by the user
Clarify the intended workflow:
- If this is meant to be automated: provide Python/shell scripts that implement the selection logic rather than asking for manual analysis
- If this is manual: restructure to make clear this is guidance for the user's own analysis, not something the command automatically does
The current phrasing makes it ambiguous whether steps 6a-6c are part of the command's automated execution or post-command guidance for the user.
225-263: Verify the batch pagination logic handles edge cases correctly.The batch processing loop (lines 220-263) uses:
START_ATincremented by 1000 each iterationTOTAL_FETCHEDcompared againstTOTALfrom the API response- Loop breaks when
TOTAL_FETCHED >= TOTALorBATCH_SIZE == 0Potential issues to verify:
- First response race condition: What if
totalin the API response changes between paginated requests? The loop compares against theTOTALfrom the first batch—if more tickets are added mid-pagination, you could miss tickets.- Batch size less than 1000: If the last batch returns fewer than 1000 tickets, the loop correctly detects completion (good). But what if an intermediate batch returns fewer than 1000 due to API quirks?
- Infinite loop protection: There's no maximum iteration limit. If the loop logic has a bug, it could retry indefinitely.
Consider adding:
- Maximum iteration limit (e.g.,
MAX_BATCHES=100with a safety check)- Explicit handling if
BATCH_SIZE < 1000mid-pagination (potential signal to stop early or warn)- Documentation of the assumption that
totaldoesn't change during pagination
157-180: Credential extraction from mcp.json uses.get()safely but lacks fallback clarity.Line 164 shows:
JIRA_EMAIL=$(jq -r '.mcpServers.atlassian.env.JIRA_USERNAME // .mcpServers.atlassian.env.JIRA_EMAIL' "$MCP_CONFIG")This uses
jq's//operator for fallback, which is good. However, the documentation should clarify:
- Which field name is authoritative? The code looks for
JIRA_USERNAMEfirst, thenJIRA_EMAIL. Should users use one or both?- Fallback behavior: Is the fallback to
JIRA_EMAILnecessary, or should the config always useJIRA_USERNAME?- Empty string handling: What if
jqreturns an empty string (valid JSON null)? The code should explicitly check for this and error gracefully.Add validation after credential extraction to fail fast if any required field is missing or empty.
141-151: Verification step references MCP tool that may not be registered.Line 142 suggests testing with:
"Use the mcp__atlassian__jira_get_issue tool to fetch OCPBUGS-1"However:
- This assumes the tool is registered and available in Claude Code
- The naming convention
mcp__atlassian__jira_get_issuemay not match actual tool names- The command explicitly states it doesn't use MCP tools—this example could confuse users about whether they need MCP setup
Consider revising this verification step to:
- Test the credential extraction from mcp.json directly (using jq)
- Test curl access manually with the stored credentials
- Remove MCP tool reference if this command doesn't actually use it
Verify that the suggested MCP tool name is accurate or provide an alternative verification method.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
plugins/jira/commands/backlog.md(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.28.0)
plugins/jira/commands/backlog.md
[high] 511-511: Discovered a potential authorization token provided in a curl command header, which could compromise the curl accessed resource.
(curl-auth-header)
🪛 LanguageTool
plugins/jira/commands/backlog.md
[style] ~384-~384: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...lready complete) - Zero comments AND very old (120+ days) - likely abandoned - Com...
(EN_WEAK_ADJECTIVE)
[style] ~529-~529: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...limits - Parallel-safe: Can process very large backlogs (10,000+ tickets) without issu...
(EN_WEAK_ADJECTIVE)
🔇 Additional comments (6)
plugins/jira/commands/backlog.md (6)
1-12: Excellent documentation structure and metadata.The frontmatter metadata, name, synopsis, and initial description sections are well-organized and clearly convey the command's purpose. The prior markdown formatting issues (language identifiers, bare URLs, list alignment) have all been corrected.
36-151: Prerequisites section is comprehensive and practical.The setup instructions are thorough and audience-friendly, covering credential configuration, MCP server setup with podman, and verification steps. The emphasis on using podman over npx, backed by testing notes, is helpful context for users.
275-348: Python processing logic is clear and well-documented.The filtering logic correctly identifies available tickets (unassigned or bot-assigned), properly normalizes priority names, and outputs grouped/statistical results. The explanation of why Python is used for batch processing (rather than in Claude context) is sound.
350-409: Intelligent ticket analysis guidance is well-reasoned.The instructions appropriately emphasize human judgment over mechanical selection, provide clear age classifications (Very Fresh through Very Old), list disqualifying and positive indicators, and define selection criteria. This strikes the right balance between automation and human expertise.
One minor style suggestion (optional, "Chill" mode): Line 384 and 529 both use "very" as an intensifier ("very old", "very large"). Consider alternatives like "significantly old" or "substantially old" / "large-scale backlogs" for variety, though this is not a blocking issue.
410-480: Output formatting and error handling are comprehensive.The example report format clearly shows how results will be organized by priority, including status, components, activity summaries, and recommendations. Error handling covers all major failure scenarios (missing credentials, invalid format, auth failures, rate limiting) with helpful user guidance.
545-584: Examples and arguments documentation provide practical clarity.The five examples cover common use cases (basic search, custom thresholds, assignee filtering, multiple projects) and include a performance validation note from testing (2,535 tickets, no 413 errors). Arguments are clearly documented with defaults and practical usage patterns.
|
This PR is from #119 but with cleaner commit history. Rebase was unsuccessful. Performed cherry pick + amend changes from @coderabbitai . |
|
Tip For best results, initiate chat on the files or code changes. @ehearne-redhat: Thank you for the context! I appreciate you taking the time to clean up the commit history. It's perfectly reasonable to use cherry-pick when a rebase becomes problematic, especially when you want to maintain a clean history. I'm glad you were able to incorporate the previous feedback. The PR looks well-structured based on the summary:
Would you like me to review the changes in this PR to ensure everything looks good with the cleaner commit history? |
|
Sure. Thank you. |
|
/assign |
Summary
/jira:backlogcommand to help developers find suitable JIRA tickets from the backlog to work onKey Features
Smart Filtering:
Intelligent Analysis:
Performance:
Comprehensive Documentation:
Command Syntax
Examples
Test plan
make lintto validate plugin structure🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
/jira:backlogcommand to find suitable Jira backlog tickets by priority and activity. Accepts a project key with optional assignee and inactivity filters; returns up to two recommended tickets per priority (Critical, High, Normal, Low) with summaries.Documentation