fix(issue-sync): find_closing_pr() reads pr:#NNN format from TODO.md (t291)#1129
fix(issue-sync): find_closing_pr() reads pr:#NNN format from TODO.md (t291)#1129marcusquinn merged 1 commit intomainfrom
Conversation
….md (t291) The function used regex 'PR #[0-9]+' which didn't match the actual 'pr:#NNN' format in TODO.md, so close comments never cited the PR even when it existed. Also: subtask IDs (t214.6) didn't match parent PR titles (t214), so GitHub search failed as a fallback. Changes: - Add Check 1: extract pr:#NNN directly from task line (most reliable) - Add Check 3b: search by parent task ID for subtask PRs - Renumber existing checks for clarity Retroactive: added PR reference comments to 36 closed issues that were missing them due to this bug. Ref #895
|
Warning Rate limit exceeded
⌛ 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. ✨ 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 |
Summary of ChangesHello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Wed Feb 11 14:36:01 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where the closing pull request was not being properly identified from TODO.md task lines. The changes introduce a new check for the pr:#NNN format and add a fallback search for parent task IDs, which effectively addresses the root cause. My review includes a few suggestions to improve shell scripting quality by simplifying command pipelines and refactoring duplicated logic, aligning with the repository's focus on high-quality shell scripting.
| # Check 1: Extract PR number from task line notes (e.g., "PR #123 merged") | ||
| # Check 1 (t291): Extract pr:#NNN from TODO.md task line — most reliable source | ||
| local todo_pr | ||
| todo_pr=$(echo "$task_line" | grep -oE 'pr:#[0-9]+' | head -1 | grep -oE '[0-9]+' || echo "") |
There was a problem hiding this comment.
This command pipeline can be simplified and made more efficient by using a single sed command to both find and extract the pull request number. This reduces the number of piped commands and sub-processes, improving performance and readability.
| todo_pr=$(echo "$task_line" | grep -oE 'pr:#[0-9]+' | head -1 | grep -oE '[0-9]+' || echo "") | |
| todo_pr=$(echo "$task_line" | sed -E -n 's/.*pr:#([0-9]+).*/\1/p' | head -1 || echo "") |
|
|
||
| # Check 2: Extract PR number from task line notes (e.g., "PR #123 merged") | ||
| local inline_pr | ||
| inline_pr=$(echo "$task_line" | grep -oiE 'PR #[0-9]+' | head -1 | grep -oE '[0-9]+' || echo "") |
There was a problem hiding this comment.
Similar to the todo_pr extraction, this command pipeline can be made more efficient. Using sed to strip non-numeric characters after the initial grep is more performant than invoking a second grep process.
| inline_pr=$(echo "$task_line" | grep -oiE 'PR #[0-9]+' | head -1 | grep -oE '[0-9]+' || echo "") | |
| inline_pr=$(echo "$task_line" | grep -oiE 'PR #[0-9]+' | head -1 | sed 's/[^0-9]*//g' || echo "") |
| # Check 3b (t291): For subtask IDs (e.g., t214.6), also search by parent ID (t214) | ||
| local parent_id | ||
| parent_id=$(echo "$task_id" | grep -oE '^t[0-9]+' || echo "") | ||
| if [[ -n "$parent_id" && "$parent_id" != "$task_id" ]]; then | ||
| pr_json=$(gh pr list --repo "$repo_slug" --state merged --search "$parent_id in:title" --limit 1 --json number,url 2>/dev/null || echo "[]") | ||
| pr_number=$(echo "$pr_json" | jq -r '.[0].number // empty' 2>/dev/null || echo "") | ||
| pr_url=$(echo "$pr_json" | jq -r '.[0].url // empty' 2>/dev/null || echo "") | ||
| if [[ -n "$pr_number" && "$pr_number" != "null" ]]; then | ||
| echo "${pr_number}|${pr_url}" | ||
| return 0 | ||
| fi | ||
| fi |
There was a problem hiding this comment.
This block of code is nearly identical to the preceding block (lines 1259-1268) that searches for a PR using task_id. To improve maintainability and avoid code duplication, consider extracting this logic into a local helper function. The function could take the search term (task_id or parent_id) as an argument and return the PR information if found.
For example:
_find_gh_pr_by_search_term() {
local search_term="$1"
local repo_slug="$2"
local pr_json
pr_json=$(gh pr list --repo "$repo_slug" --state merged --search "$search_term in:title" --limit 1 --json number,url 2>/dev/null || echo "[]")
local pr_number
pr_number=$(echo "$pr_json" | jq -r '.[0].number // empty' 2>/dev/null || echo "")
local pr_url
pr_url=$(echo "$pr_json" | jq -r '.[0].url // empty' 2>/dev/null || echo "")
if [[ -n "$pr_number" && "$pr_number" != "null" ]]; then
echo "${pr_number}|${pr_url}"
return 0
fi
return 1
}You could then call this function for both task_id and parent_id to reduce the duplicated code.
1 participant
Summary
find_closing_pr()to read thepr:#NNNformat from TODO.md task lines — the most reliable PR sourcet214.6-> searchest214)Root Cause
find_closing_pr()had two checks:PR #[0-9]+— didn't match the actualpr:#963format (lowercase, no space)t214.6don't match parent PR titles like(t214)This meant close comments said "Completed. Task tXXX marked done in TODO.md." without citing the PR, even when
pr:#NNNexisted in TODO.md.Changes
.agents/scripts/issue-sync-helper.shTesting
bash -nsyntax check: passshellcheck -S warning: zero violationspr:#963from real TODO.md task linet214.6->t214Ref #895