t1268: auto-detect plan refs from PLANS.md Task/TODO field in compose_issue_body#2000
t1268: auto-detect plan refs from PLANS.md Task/TODO field in compose_issue_body#2000marcusquinn merged 1 commit intomainfrom
Conversation
…issue_body (t1268) - Add find_plan_by_task_id(): scans PLANS.md for **TODO:** or **Task:** fields matching the task ID; supports subtask walk-up (t004.2 → t004) - Add extract_plan_extra_sections(): includes Context, Research, Architecture, Tool Matrix, Linkage, Phases, Risks, Open Questions, and other subsections beyond the 4 standard extractors - Update compose_issue_body(): when plan_link is empty, auto-detect via find_plan_by_task_id() before building the metadata header - Extract plan ID (p029, p025, etc.) from TOON block for both explicit links and auto-detected plans; include as | **Plan:** `pNNN` in metadata header - Expand plan context sections: Context & Architecture (collapsed), Progress, Decision Log, Discoveries — all populated from auto-detected plan - enrich command automatically benefits: compose_issue_body() now enriches all tasks with matching PLANS.md entries, no manual → [...] links required
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 enhances the Highlights
Changelog
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
|
WalkthroughThe PR adds plan detection and context enrichment to the issue sync helper script. Two new functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 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 |
🔍 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: Fri Feb 20 03:41:50 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.agents/scripts/issue-sync-helper.sh (3)
2858-2880:cmd_parsedoesn't use auto-detect, so debug output is incomplete vs the composed body.The
=== Plan Section ===block (lines 2858-2880) only shows plan context when an explicitplan_linkis present. Butcompose_issue_body(called at line 2887) now auto-detects plans viafind_plan_by_task_id. A user runningparse tNNNwould see plan content in the composed body at the bottom but no plan section in the debug output above — potentially confusing when debugging auto-detect behavior.♻️ Add auto-detect fallback to cmd_parse
if [[ -n "$plan_link" ]]; then echo "=== Plan Section ===" local plan_section plan_section=$(extract_plan_section "$plan_link" "$project_root") if [[ -n "$plan_section" ]]; then echo "Purpose:" extract_plan_purpose "$plan_section" echo "" echo "Decisions:" extract_plan_decisions "$plan_section" echo "" echo "Progress:" extract_plan_progress "$plan_section" else echo "(no plan section found for anchor: $plan_link)" fi echo "" + else + # Auto-detect plan from PLANS.md **TODO:**/**Task:** fields + local auto_detected + auto_detected=$(find_plan_by_task_id "$task_id" "$project_root") + if [[ -n "$auto_detected" ]]; then + echo "=== Plan Section (auto-detected) ===" + local detected_plan_id + detected_plan_id=$(echo "$auto_detected" | head -1) + local plan_section + plan_section=$(echo "$auto_detected" | tail -n +2) + [[ -n "$detected_plan_id" ]] && echo "Plan ID: $detected_plan_id" + echo "Purpose:" + extract_plan_purpose "$plan_section" + echo "" + echo "Decisions:" + extract_plan_decisions "$plan_section" + echo "" + echo "Progress:" + extract_plan_progress "$plan_section" + echo "" + fi fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/issue-sync-helper.sh around lines 2858 - 2880, The debug output in cmd_parse only prints a plan section when parse_task_line yields an explicit plan_link, which omits plans auto-detected later by compose_issue_body via find_plan_by_task_id; update cmd_parse so that if plan_link is empty it attempts the same auto-detect fallback (call find_plan_by_task_id or compose_issue_body's detection logic) to set plan_link and then call extract_plan_section/ extract_plan_purpose/ extract_plan_decisions/ extract_plan_progress as before; reference the existing variables and functions plan_link, parse_task_line, extract_plan_section, compose_issue_body, and find_plan_by_task_id to mirror the composed-body behavior and ensure debug output matches what compose_issue_body will produce.
1344-1353: Leading blank lines in output when first extra section is found.When
resultis empty and the first heading matches, the concatenation produces\n\n**Heading**\n\n...— two leading blank lines. Inside the<details>tag at line 1678 this renders as extra whitespace before the first section.✨ Trim leading whitespace
- if [[ -n "$content" ]]; then - result="${result}"$'\n\n'"**${heading}**"$'\n\n'"${content}" + if [[ -n "$content" ]]; then + if [[ -n "$result" ]]; then + result="${result}"$'\n\n'"**${heading}**"$'\n\n'"${content}" + else + result="**${heading}**"$'\n\n'"${content}" + fi fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/issue-sync-helper.sh around lines 1344 - 1353, The concatenation always prefixes "\n\n" which adds two leading blank lines when result is empty; fix the loop that builds result (using the extra_headings loop and _extract_plan_subsection call) so it only prepends the "\n\n" separator for subsequent sections and not for the first match—i.e., check if result is empty before adding the separator and append "**${heading}**\n\n${content}" for the first match, otherwise use the "\n\n**${heading}**\n\n${content}" form.
1598-1601: Misleading comment: plan ID is shown for both explicit and auto-detected plans.The comment on line 1598 says "when auto-detected (no explicit link)" but
detected_plan_idis also populated for explicit plan links (lines 1569-1571). The behavior is correct — the comment just doesn't reflect it.📝 Fix comment
- # Include plan ID in metadata header when auto-detected (no explicit link) + # Include plan ID in metadata header when available (explicit or auto-detected)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.agents/scripts/issue-sync-helper.sh around lines 1598 - 1601, The comment above the block that appends the plan ID is misleading: update the comment near the block that checks detected_plan_id (the if [[ -n "$detected_plan_id" ]] { ... } section) to state that the plan ID is included for both auto-detected and explicitly linked plans (since detected_plan_id is set for explicit links earlier), so replace the "when auto-detected (no explicit link)" wording with a neutral description like "include plan ID in metadata header when available."
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.agents/scripts/issue-sync-helper.sh:
- Around line 1674-1679: The markdown inside the <details> block is missing a
blank line after </summary>, so update the body construction where
extra_sections is appended (the block that sets
extra_sections=$(extract_plan_extra_sections "$plan_section") and then builds
body) to use $'\n\n' immediately after "</summary>" instead of $'\n' so the
generated string includes a blank line between </summary> and the markdown
content; ensure the concatenation that builds body for the Plan: Context &
Architecture details matches the other similar blocks that use $'\n\n'.
- Line 1264: The grep pattern in the match_line assignment uses basic-regex
alternation \(TODO\|Task\) which fails on macOS BSD grep; update the command
that sets match_line to use extended regex (grep -E or egrep) and replace the
BRE alternation with a POSIX ERE group, e.g. use grep -E
"^\*\*(TODO|Task):\*\*.*\b${lookup_id}\b" "$plans_file" so the
lookup_id/plans_file/match_line logic continues to work on macOS.
---
Nitpick comments:
In @.agents/scripts/issue-sync-helper.sh:
- Around line 2858-2880: The debug output in cmd_parse only prints a plan
section when parse_task_line yields an explicit plan_link, which omits plans
auto-detected later by compose_issue_body via find_plan_by_task_id; update
cmd_parse so that if plan_link is empty it attempts the same auto-detect
fallback (call find_plan_by_task_id or compose_issue_body's detection logic) to
set plan_link and then call extract_plan_section/ extract_plan_purpose/
extract_plan_decisions/ extract_plan_progress as before; reference the existing
variables and functions plan_link, parse_task_line, extract_plan_section,
compose_issue_body, and find_plan_by_task_id to mirror the composed-body
behavior and ensure debug output matches what compose_issue_body will produce.
- Around line 1344-1353: The concatenation always prefixes "\n\n" which adds two
leading blank lines when result is empty; fix the loop that builds result (using
the extra_headings loop and _extract_plan_subsection call) so it only prepends
the "\n\n" separator for subsequent sections and not for the first match—i.e.,
check if result is empty before adding the separator and append
"**${heading}**\n\n${content}" for the first match, otherwise use the
"\n\n**${heading}**\n\n${content}" form.
- Around line 1598-1601: The comment above the block that appends the plan ID is
misleading: update the comment near the block that checks detected_plan_id (the
if [[ -n "$detected_plan_id" ]] { ... } section) to state that the plan ID is
included for both auto-detected and explicitly linked plans (since
detected_plan_id is set for explicit links earlier), so replace the "when
auto-detected (no explicit link)" wording with a neutral description like
"include plan ID in metadata header when available."
| # Match: **TODO:** t004, t007 OR **Task:** t004 | ||
| # The task ID must appear as a whole word (not as prefix of another ID) | ||
| local match_line match_line_num | ||
| match_line=$(grep -n "^\*\*\(TODO\|Task\):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, check the specific line 1264 and surrounding context
sed -n '1260,1270p' .agents/scripts/issue-sync-helper.shRepository: marcusquinn/aidevops
Length of output: 554
🏁 Script executed:
# Search for all uses of \| in the script to find similar issues
rg -n '\\\\|' .agents/scripts/issue-sync-helper.shRepository: marcusquinn/aidevops
Length of output: 50379
🏁 Script executed:
# Check if the script mentions macOS or BSD compatibility requirements
rg -n -i 'macos|bsd|darwin|brew' .agents/scripts/issue-sync-helper.sh | head -20Repository: marcusquinn/aidevops
Length of output: 135
🏁 Script executed:
# Verify the complete find_plan_by_task_id function signature and usage
sed -n '1239,1320p' .agents/scripts/issue-sync-helper.shRepository: marcusquinn/aidevops
Length of output: 2689
Fix grep portability: \| BRE alternation breaks on macOS BSD grep.
Line 1264 uses basic regex \(TODO\|Task\) which is a GNU extension unsupported by macOS's BSD grep—the command will silently match zero lines. Since this script targets macOS users (brew install gh), this breaks plan auto-detection on the primary platform.
Switch to extended regex mode:
🐛 Proposed fix
- match_line=$(grep -n "^\*\*\(TODO\|Task\):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true)
+ match_line=$(grep -nE "^\*\*(TODO|Task):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true)📝 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.
| match_line=$(grep -n "^\*\*\(TODO\|Task\):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true) | |
| match_line=$(grep -nE "^\*\*(TODO|Task):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/issue-sync-helper.sh at line 1264, The grep pattern in the
match_line assignment uses basic-regex alternation \(TODO\|Task\) which fails on
macOS BSD grep; update the command that sets match_line to use extended regex
(grep -E or egrep) and replace the BRE alternation with a POSIX ERE group, e.g.
use grep -E "^\*\*(TODO|Task):\*\*.*\b${lookup_id}\b" "$plans_file" so the
lookup_id/plans_file/match_line logic continues to work on macOS.
| # Extra sections: Context, Research, Architecture, Tool Matrix, etc. (collapsed) | ||
| local extra_sections | ||
| extra_sections=$(extract_plan_extra_sections "$plan_section") | ||
| if [[ -n "$extra_sections" ]]; then | ||
| body="$body"$'\n\n'"<details><summary>Plan: Context & Architecture</summary>"$'\n'"$extra_sections"$'\n\n'"</details>" | ||
| fi |
There was a problem hiding this comment.
Missing blank line after </summary> — markdown won't render inside <details> on GitHub.
Lines 1685, 1692, 1699 correctly use $'\n\n' (blank line) after </summary>, but line 1678 uses only $'\n'. GitHub requires a blank line between </summary> and the markdown content for it to render properly. Currently this works by accident because extra_sections starts with \n\n (see earlier comment), but that coupling is fragile.
🐛 Proposed fix
- body="$body"$'\n\n'"<details><summary>Plan: Context & Architecture</summary>"$'\n'"$extra_sections"$'\n\n'"</details>"
+ body="$body"$'\n\n'"<details><summary>Plan: Context & Architecture</summary>"$'\n\n'"$extra_sections"$'\n\n'"</details>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.agents/scripts/issue-sync-helper.sh around lines 1674 - 1679, The markdown
inside the <details> block is missing a blank line after </summary>, so update
the body construction where extra_sections is appended (the block that sets
extra_sections=$(extract_plan_extra_sections "$plan_section") and then builds
body) to use $'\n\n' immediately after "</summary>" instead of $'\n' so the
generated string includes a blank line between </summary> and the markdown
content; ensure the concatenation that builds body for the Plan: Context &
Architecture details matches the other similar blocks that use $'\n\n'.
![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
The pull request introduces auto-detection of plan references from PLANS.md based on task IDs, which significantly improves the richness of GitHub issue bodies without requiring manual linking. The implementation includes subtask walk-up support and extraction of additional plan sections. My feedback focuses on improving the robustness of task ID matching, reducing code duplication through helper functions, and refining the formatting of extracted sections.
| # Match: **TODO:** t004, t007 OR **Task:** t004 | ||
| # The task ID must appear as a whole word (not as prefix of another ID) | ||
| local match_line match_line_num | ||
| match_line=$(grep -n "^\*\*\(TODO\|Task\):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true) |
There was a problem hiding this comment.
The use of \b for word boundaries in the grep pattern is problematic because . is considered a boundary character. This means that searching for t004 will incorrectly match t004.2, violating the 'whole word' requirement stated in the comment. A more precise boundary check that includes the dot character is needed. Additionally, for robustness when extracting metadata, tail -1 should be preferred over head -1.
| match_line=$(grep -n "^\*\*\(TODO\|Task\):\*\*.*\b${lookup_id}\b" "$plans_file" | head -1 || true) | |
| match_line=$(grep -nE "^\*\*(TODO|Task):\*\*.*(^|[^a-zA-Z0-9.])${lookup_id}($|[^a-zA-Z0-9.])" "$plans_file" | tail -1 || true) |
References
- When extracting metadata fields from a line using 'grep' and 'sed' in shell scripts, use 'tail -1' instead of 'head -1' to ensure robustness, especially when the field might appear multiple times or be at the end of the line.
| plan_id=$(awk -v start="$heading_num" ' | ||
| NR < start { next } | ||
| NR > start && /^### / { exit } | ||
| /^<!--TOON:plan\{/ { | ||
| # Extract first field (plan ID) from TOON data line | ||
| getline data_line | ||
| if (match(data_line, /^p[0-9]+,/)) { | ||
| id = substr(data_line, RSTART, RLENGTH - 1) | ||
| print id | ||
| exit | ||
| } | ||
| } | ||
| ' "$plans_file" || true) |
There was a problem hiding this comment.
The logic for extracting a plan ID from a TOON block is duplicated here and in compose_issue_body. It should be extracted into a shared internal helper function to improve maintainability and ensure consistency.
References
- Extract repeated logic into an internal helper function to improve maintainability.
| local content | ||
| content=$(_extract_plan_subsection "$plan_section" "$heading" 0 "true") | ||
| if [[ -n "$content" ]]; then | ||
| result="${result}"$'\n\n'"**${heading}**"$'\n\n'"${content}" |
There was a problem hiding this comment.
The string concatenation here results in leading newlines if result is initially empty. While not critical, it can lead to inconsistent spacing in the final issue body. It's better to only prepend newlines when appending to an existing result.
| result="${result}"$'\n\n'"**${heading}**"$'\n\n'"${content}" | |
| if [[ -z "$result" ]]; then result="**${heading}**"$'\n\n'"${content}"; else result="${result}"$'\n\n'"**${heading}**"$'\n\n'"${content}"; fi |
| detected_plan_id=$(echo "$plan_section" | awk ' | ||
| /^<!--TOON:plan\{/ { getline data; if (match(data, /^p[0-9]+,/)) { print substr(data, RSTART, RLENGTH-1); exit } } | ||
| ' || true) |
There was a problem hiding this comment.
This awk script duplicates the plan ID extraction logic found in find_plan_by_task_id. As suggested previously, this should be refactored into a helper function.
References
- Extract repeated logic into an internal helper function to improve maintainability.
marcusquinn
dismissed
coderabbitai[bot]’s stale review
Auto-dismissed: bot review does not block autonomous pipeline
1 participant
Auto-detect plan references from PLANS.md
Task:/TODO:field and compose rich issue bodies.Changes
find_plan_by_task_id(): scans PLANS.md for**TODO:**or**Task:**fields matching the task ID; supports subtask walk-up (t004.2 → t004)extract_plan_extra_sections(): includes Context, Research, Architecture, Tool Matrix, Linkage, Phases, Risks, Open Questions beyond the 4 standard extractorscompose_issue_body(): whenplan_linkis empty, auto-detects viafind_plan_by_task_id()before building the metadata header| **Plan:** \pNNN`` in metadata headerenrichcommand automatically benefits — no manual→ [...]links requiredRef #1999
Summary by CodeRabbit