Conversation
…est templates - Add cmd_upgrade_planning function with --dry-run, --force, --no-backup options - Detects minimal templates (missing TOON markers) and offers upgrade - Preserves existing tasks by extracting and merging into new structure - Creates backups before modification - Updates .aidevops.json with templates_version tracking - Add command to help, main entry point, and documentation
WalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant User
participant CLI as aidevops.sh
participant Git as Git Repo
participant FS as File System
participant Config as .aidevops.json
User->>CLI: aidevops upgrade-planning [--dry-run|--force|--no-backup]
CLI->>CLI: Parse flags, validate planning feature
CLI->>Git: Ensure repo initialized & check branch protection
Git-->>CLI: Status OK / protected
alt Protected branch
CLI->>User: Prompt to create worktree or proceed
User-->>CLI: Choice
CLI->>Git: Create/switch worktree (if chosen)
end
CLI->>FS: Locate template files and existing TODO/PLANS
FS-->>CLI: Templates and files present/missing
CLI->>CLI: Determine if upgrade needed (TOON markers)
alt Upgrade Needed
CLI->>CLI: If dry-run -> prepare preview
alt Not dry-run
CLI->>FS: Create backups (unless --no-backup)
FS-->>CLI: Backups created
CLI->>CLI: Extract tasks and merge into new template sections
CLI->>FS: Write new template-based files
FS-->>CLI: Files written
CLI->>Config: Update templates_version in .aidevops.json
Config-->>CLI: Confirm update
end
CLI->>User: Report/preview results and next steps
else No upgrade required
CLI->>User: Notify no changes needed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 introduces a significant enhancement to the Highlights
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: Tue Jan 13 01:04:23 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 introduces a new upgrade-planning command to help users migrate their existing TODO.md and PLANS.md files to the latest TOON-enhanced templates. The implementation is comprehensive, including backups, dry-run, and force options. The documentation has also been updated accordingly.
My review focuses on the shell script implementation. I've found several critical issues related to file manipulation using sed and awk that could lead to file corruption. Specifically, the logic for merging existing tasks/plans into the new templates is flawed and will likely break the structure of the TOON blocks. Additionally, there are several places where JSON files are being modified with sed and grep, which is fragile; using jq (which is already a dependency) would be much safer and more robust. I've provided detailed comments and suggestions for these issues.
| awk -v tasks="$existing_tasks" ' | ||
| /<!--TOON:backlog/ { print; getline; print; print ""; print tasks; next } | ||
| { print } | ||
| ' "$todo_file" > "$temp_file" | ||
| mv "$temp_file" "$todo_file" |
There was a problem hiding this comment.
The awk script for merging tasks into TODO.md is likely to corrupt the file. It appears to inject the existing_tasks inside the multi-line <!--TOON:backlog...--> block. The awk script matches the opening line of the block, prints it, prints the next line using getline, and then inserts the tasks. This will break the TOON block's structure, as the old tasks are not in TOON format.
The tasks should likely be inserted in the human-readable part of the file, after the TOON block has been closed with -->.
| awk -v plans="$existing_plans" ' | ||
| /<!--TOON:active_plans/ { print; getline; print; print ""; print plans; next } | ||
| { print } | ||
| ' "$plans_file" > "$temp_file" | ||
| mv "$temp_file" "$plans_file" |
There was a problem hiding this comment.
Similar to the issue in TODO.md processing, this awk script for merging plans into PLANS.md is likely to corrupt the file. It injects the existing_plans inside the <!--TOON:active_plans...--> block, which is probably not the intention.
The plans should be inserted outside this block. A safer approach would be to find the closing tag (-->) of the <!--TOON:active_plans...--> block and insert the plans after it.
aidevops.sh
Outdated
| local planning_enabled | ||
| planning_enabled=$(grep -o '"planning": *true' "$project_root/.aidevops.json" 2>/dev/null || echo "") | ||
| if [[ -z "$planning_enabled" ]]; then | ||
| print_error "Planning feature not enabled" | ||
| print_info "Run 'aidevops init planning' to enable" | ||
| return 1 | ||
| fi |
There was a problem hiding this comment.
Using grep to parse JSON is fragile and can lead to incorrect behavior if the formatting of .aidevops.json changes (e.g., different spacing or line breaks). Since jq is already a dependency of this script (used in get_remote_version), it's better to use it for robustly checking the value of the planning feature.
| local planning_enabled | |
| planning_enabled=$(grep -o '"planning": *true' "$project_root/.aidevops.json" 2>/dev/null || echo "") | |
| if [[ -z "$planning_enabled" ]]; then | |
| print_error "Planning feature not enabled" | |
| print_info "Run 'aidevops init planning' to enable" | |
| return 1 | |
| fi | |
| # Check if planning is enabled | |
| if ! jq -e '.features.planning == true' "$project_root/.aidevops.json" &>/dev/null; then | |
| print_error "Planning feature not enabled" | |
| print_info "Run 'aidevops init planning' to enable" | |
| return 1 | |
| fi |
aidevops.sh
Outdated
| sed '1,/^---$/d' "$todo_template" | sed '1,/^---$/d' > "$todo_file" 2>/dev/null || \ | ||
| cp "$todo_template" "$todo_file" |
There was a problem hiding this comment.
The use of a double sed '1,/^---$/d' pipe to strip frontmatter is confusing and potentially buggy. If the template has only one frontmatter block, the second sed is redundant. If the content of the template after the first frontmatter block contains a --- line, the second sed command will incorrectly truncate the content. A single sed is sufficient and safer if the goal is to remove one frontmatter block.
| sed '1,/^---$/d' "$todo_template" | sed '1,/^---$/d' > "$todo_file" 2>/dev/null || \ | |
| cp "$todo_template" "$todo_file" | |
| sed '1,/^---$/d' "$todo_template" > "$todo_file" 2>/dev/null || \ | |
| cp "$todo_template" "$todo_file" |
aidevops.sh
Outdated
| sed '1,/^---$/d' "$plans_template" | sed '1,/^---$/d' > "$plans_file" 2>/dev/null || \ | ||
| cp "$plans_template" "$plans_file" |
There was a problem hiding this comment.
The use of a double sed '1,/^---$/d' pipe to strip frontmatter is confusing and potentially buggy, just like in the TODO.md processing. If the template has only one frontmatter block, the second sed is redundant. If the content of the template after the first frontmatter block contains a --- line, the second sed command will incorrectly truncate the content. A single sed is sufficient and safer if the goal is to remove one frontmatter block.
| sed '1,/^---$/d' "$plans_template" | sed '1,/^---$/d' > "$plans_file" 2>/dev/null || \ | |
| cp "$plans_template" "$plans_file" | |
| sed '1,/^---$/d' "$plans_template" > "$plans_file" 2>/dev/null || \ | |
| cp "$plans_template" "$plans_file" |
aidevops.sh
Outdated
| # Add templates_version to config if not present | ||
| if ! grep -q '"templates_version"' "$config_file" 2>/dev/null; then | ||
| # Insert templates_version after version line | ||
| sed -i.tmp "s/\"version\": \"[^\"]*\"/\"version\": \"$aidevops_version\",\n \"templates_version\": \"$aidevops_version\"/" "$config_file" 2>/dev/null || true | ||
| rm -f "${config_file}.tmp" | ||
| else | ||
| # Update existing templates_version | ||
| sed -i.tmp "s/\"templates_version\": \"[^\"]*\"/\"templates_version\": \"$aidevops_version\"/" "$config_file" 2>/dev/null || true | ||
| rm -f "${config_file}.tmp" | ||
| fi |
There was a problem hiding this comment.
Using sed to modify the .aidevops.json file is fragile and can easily lead to invalid JSON, for example if the version key has a trailing comma. Since jq is a dependency of this script, it should be used to safely add or update the templates_version key. This also ensures the main version is updated consistently.
| # Add templates_version to config if not present | |
| if ! grep -q '"templates_version"' "$config_file" 2>/dev/null; then | |
| # Insert templates_version after version line | |
| sed -i.tmp "s/\"version\": \"[^\"]*\"/\"version\": \"$aidevops_version\",\n \"templates_version\": \"$aidevops_version\"/" "$config_file" 2>/dev/null || true | |
| rm -f "${config_file}.tmp" | |
| else | |
| # Update existing templates_version | |
| sed -i.tmp "s/\"templates_version\": \"[^\"]*\"/\"templates_version\": \"$aidevops_version\"/" "$config_file" 2>/dev/null || true | |
| rm -f "${config_file}.tmp" | |
| fi | |
| # Add/Update templates_version to config | |
| local temp_json_file="${config_file}.tmp" | |
| jq --arg version "$aidevops_version" ' .version = $version | .templates_version = $version ' "$config_file" > "$temp_json_file" && mv "$temp_json_file" "$config_file" |
🤖 Augment PR SummarySummary: Adds an Changes:
Technical Notes: Template application strips frontmatter and replaces 🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
aidevops.sh
Outdated
| # Add templates_version to config if not present | ||
| if ! grep -q '"templates_version"' "$config_file" 2>/dev/null; then | ||
| # Insert templates_version after version line | ||
| sed -i.tmp "s/\"version\": \"[^\"]*\"/\"version\": \"$aidevops_version\",\n \"templates_version\": \"$aidevops_version\"/" "$config_file" 2>/dev/null || true |
There was a problem hiding this comment.
The sed substitution here relies on \n in the replacement, which works on GNU sed but on macOS/BSD sed it typically writes a literal \n, potentially corrupting .aidevops.json. Because errors are suppressed (2>/dev/null || true), this could fail silently and leave invalid JSON behind.
🤖 Was this useful? React with 👍 or 👎
aidevops.sh
Outdated
| fi | ||
|
|
||
| # Copy template (strip frontmatter) | ||
| sed '1,/^---$/d' "$todo_template" | sed '1,/^---$/d' > "$todo_file" 2>/dev/null || \ |
There was a problem hiding this comment.
The frontmatter stripping pipeline (sed '1,/^---$/d' | sed '1,/^---$/d') will output an empty file if the template ever lacks --- markers, and the || cp ... fallback won’t run because sed still exits 0. That could overwrite planning files with empty content (especially risky when --no-backup is used).
🤖 Was this useful? React with 👍 or 👎
![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)
aidevops.sh (3)
771-778: Consider warning on unknown arguments.Unknown arguments are silently ignored (
*) shift ;;), which could mask typos like--forseinstead of--force. Consider adding a warning:♻️ Optional: warn on unknown arguments
--force|-f) force=true; shift ;; --no-backup) backup=false; shift ;; --dry-run|-n) dry_run=true; shift ;; - *) shift ;; + *) + print_warning "Unknown option: $1" + shift + ;; esac
1154-1157: Consider alias clarity:upcould be confused withupdate.The
upalias forupgrade-planningmight cause confusion sinceuis already an alias forupdate. Users might expectupto mean "update" as well. Consider usingugpor keeping only the full command name.♻️ Optional: Use a more distinct alias
- upgrade-planning|up) + upgrade-planning|ugp) shift cmd_upgrade_planning "$@" ;;
1010-1013: Add explicit return at end of function.Per coding guidelines, shell scripts should use explicit returns. The function exits implicitly after the final echo statements. Add
return 0for clarity:♻️ Add explicit return
echo "If issues occurred, restore from backups:" [[ "$todo_needs_upgrade" == "true" ]] && echo " mv TODO.md.bak TODO.md" [[ "$plans_needs_upgrade" == "true" ]] && echo " mv todo/PLANS.md.bak todo/PLANS.md" + + return 0 }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.agent/AGENTS.mdREADME.mdaidevops.sh
🧰 Additional context used
📓 Path-based instructions (2)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Use placeholders in examples and note secure storage location in documentation
Files:
README.md
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
**/*.sh: Use local var="$1" pattern for shell variable assignments
Use explicit returns in shell scripts
Run ShellCheck on all scripts with zero violations required
Files:
aidevops.sh
🧠 Learnings (8)
📚 Learning: 2025-11-29T04:34:27.158Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-29T04:34:27.158Z
Learning: All instructions, documentation, and operational guidance should be maintained in AGENTS.md as the single source of truth
Applied to files:
.agent/AGENTS.md
📚 Learning: 2025-11-29T04:34:30.742Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-11-29T04:34:30.742Z
Learning: Maintain all instructions, documentation, and operational guidance in AGENTS.md as the single source of truth
Applied to files:
.agent/AGENTS.md
📚 Learning: 2025-11-29T04:34:42.033Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-29T04:34:42.033Z
Learning: Maintain all AI assistant instructions, documentation, and operational guidance in AGENTS.md as the single source of truth
Applied to files:
.agent/AGENTS.md
📚 Learning: 2025-11-29T04:34:42.033Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-29T04:34:42.033Z
Learning: Reference AGENTS.md (authoritative) instead of AGENT.md for AI assistant guidance
Applied to files:
.agent/AGENTS.md
📚 Learning: 2025-11-29T04:34:30.742Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-11-29T04:34:30.742Z
Learning: Reference AGENTS.md for authoritative AI assistant guidance instead of GEMINI.md
Applied to files:
.agent/AGENTS.md
📚 Learning: 2026-01-06T15:57:56.008Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T15:57:56.008Z
Learning: Applies to **/AGENTS.md : Limit root AGENTS.md to ~50-100 max instructions with universal applicability to >80% of tasks
Applied to files:
.agent/AGENTS.md
📚 Learning: 2025-12-22T01:24:53.937Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: templates/home/AGENTS.md:0-0
Timestamp: 2025-12-22T01:24:53.937Z
Learning: Reference the authoritative repository at ~/Git/aidevops/ for all detailed AI assistant instructions and configurations
Applied to files:
.agent/AGENTS.md
📚 Learning: 2025-12-22T01:25:03.138Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: templates/home/git/AGENTS.md:0-0
Timestamp: 2025-12-22T01:25:03.138Z
Learning: Use standardized patterns as defined in the authoritative DevOps repository at ~/Git/aidevops/
Applied to files:
.agent/AGENTS.md
🧬 Code graph analysis (1)
aidevops.sh (1)
setup.sh (4)
print_error(29-29)print_info(26-26)print_warning(28-28)print_success(27-27)
🪛 LanguageTool
README.md
[grammar] ~148-~148: Ensure spelling is correct
Context: ...eads` ### Upgrade Planning Files When aidevops templates evolve, upgrade existing proj...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (8)
README.md (1)
146-157: LGTM! Clear documentation for the new upgrade-planning command.The new section follows the existing README patterns, provides practical examples with all flag variants (
--dry-run,--force), and clearly explains the TOON-enhanced upgrade behavior. Good addition to the user documentation..agent/AGENTS.md (1)
434-434: LGTM! CLI reference table updated correctly.The new command entry follows the existing table format and maintains AGENTS.md as the authoritative source for CLI documentation. Based on learnings, this is the correct place for such updates.
aidevops.sh (6)
783-806: LGTM! Solid validation with clear error messages.The git repository validation, initialization check, and planning feature verification all provide explicit returns and helpful guidance messages. This follows the pattern of defensive checking before proceeding.
808-821: LGTM! Template validation with clear paths.Template existence checks are properly implemented with explicit error returns and full path information for debugging.
823-863: LGTM! Clean upgrade detection logic.The TOON marker detection with
grep -q "TOON:meta"is a reasonable heuristic for identifying minimal vs. enhanced templates. The early exit onneeds_upgrade=falseavoids unnecessary work.
867-873: LGTM! Dry-run implementation is correct.Exits cleanly after showing what would be upgraded, with no file modifications. Explicit
return 0.
928-932: LGTM! Task merging logic preserves existing work.The awk-based merge correctly identifies the TOON marker and inserts preserved tasks at the appropriate location. Using a temp file with
mvensures atomic updates.
1091-1104: LGTM! Help text follows established patterns.The new command documentation is well-integrated into the help output, with clear description and example usage.
- Use jq for JSON parsing when available (fallback to grep) - Fix awk scripts to insert tasks AFTER TOON block closing tag - Use cleaner awk for YAML frontmatter stripping - Use jq for updating .aidevops.json (fallback to sed)
🔍 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: Tue Jan 13 01:09:34 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @aidevops.sh:
- Around line 1003-1012: The sed replacement using "\n" is incompatible with BSD
sed and produces malformed JSON; update the fallback to use a portable awk-based
edit instead of the multi-line sed: when config_file lacks "templates_version"
use an awk script that locates the line matching /"version":/ and prints that
line followed by a new indented `"templates_version": "` aidevops_version `",`
then prints the remaining lines (replace the sed invocation that inserts
templates_version), and when updating an existing "templates_version" keep using
a safe single-line sed substitution or an awk in-place rewrite that replaces the
"templates_version" value with the new aidevops_version; reference the variables
config_file and aidevops_version and remove reliance on "\n" in sed.
- Around line 968-969: The awk invocation that processes PLANS.md (the line
invoking awk with skip and /^---$/ and redirecting to "$plans_file") has the
same frontmatter bug as TODO.md; update that awk command to use the corrected
frontmatter pattern used for TODO.md so it properly skips the initial
YAML/frontmatter block (replace the current /^---$/ handling with the fixed
pattern and keep the fallback cp "$plans_template" "$plans_file" behavior
intact).
- Around line 922-923: The current awk command that processes "$todo_template"
resets skip to 0 after the closing '---' causing skip<2 to always true and thus
skipping all content; replace that awk invocation with logic that increments a
counter on lines matching ^---$, does not reset it, and only prints lines once
the counter is >= 2 (i.e., skip until you have seen two '---' markers), or
replace the whole pipeline with the suggested sed frontmatter-stripping pattern
that quits if the file does not start with '---' and deletes through the second
'---', writing the result to "$todo_file".
🧹 Nitpick comments (2)
aidevops.sh (2)
771-778: Consider warning on unknown arguments.The
*) shift ;;silently ignores unrecognized options, which could mask typos like--dryruninstead of--dry-run. A warning would improve UX.💡 Optional: Add warning for unknown arguments
--force|-f) force=true; shift ;; --no-backup) backup=false; shift ;; --dry-run|-n) dry_run=true; shift ;; - *) shift ;; + *) + if [[ "$1" == -* ]]; then + print_warning "Unknown option: $1" + fi + shift + ;; esac
930-942: Potential escape sequence issues in awk variable.The
-v tasks="$existing_tasks"assignment causes awk to interpret escape sequences (e.g.,\n,\t,\\). If tasks contain backslashes, they may be corrupted. Consider usingENVIRONfor literal string passing.♻️ Safer approach using ENVIRON
if grep -q "<!--TOON:backlog" "$todo_file"; then local temp_file="${todo_file}.merge" - awk -v tasks="$existing_tasks" ' + TASKS="$existing_tasks" awk ' /<!--TOON:backlog/ { in_backlog=1 } - in_backlog && /^-->$/ { print; print ""; print tasks; in_backlog=0; next } + in_backlog && /^-->$/ { print; print ""; print ENVIRON["TASKS"]; in_backlog=0; next } { print } ' "$todo_file" > "$temp_file"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aidevops.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
**/*.sh: Use local var="$1" pattern for shell variable assignments
Use explicit returns in shell scripts
Run ShellCheck on all scripts with zero violations required
Files:
aidevops.sh
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-29T04:34:42.033Z
Learning: Maintain all AI assistant instructions, documentation, and operational guidance in AGENTS.md as the single source of truth
🧬 Code graph analysis (1)
aidevops.sh (1)
setup.sh (4)
print_error(29-29)print_info(26-26)print_warning(28-28)print_success(27-27)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
aidevops.sh (4)
783-790: LGTM!Git repository detection and project root extraction follow best practices with proper error handling and
localvariable usage.
799-814: LGTM!The jq-with-grep-fallback pattern for checking
.features.planningis robust. Thejq -eflag correctly propagates boolean evaluation to exit status.
1167-1170: LGTM!The routing and argument forwarding for
upgrade-planningis correct. Theupalias is intuitive for "upgrade-planning" (distinct fromufor "update").
875-900: LGTM!The dry-run handling, backup creation, and confirmation flow are well-implemented. Safe defaults (backup enabled, confirmation required) protect users from accidental data loss.
- Add check_protected_branch() helper function - Offers worktree creation when on main/master - Options: create worktree, continue on main, or cancel - Skips check for upgrade-planning --dry-run - Uses worktree-helper.sh if available, falls back to git worktree
🔍 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: Tue Jan 13 01:31:00 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @aidevops.sh:
- Around line 1116-1123: The restore hints are being printed regardless of the
--no-backup option; wrap the two conditional echo lines that print "mv
TODO.md.bak ..." and "mv todo/PLANS.md.bak ..." (the checks using
todo_needs_upgrade and plans_needs_upgrade) in an additional guard that ensures
backups were actually created (check the backup flag variable, e.g., test
"$backup" == "true" or equivalent used by the script) so the restore suggestions
are only shown when backup is enabled.
- Around line 1100-1110: The sed replacement using a literal "\n" in the block
that updates/ inserts "templates_version" is not portable to BSD/macOS sed;
update the two sed invocations that target "$config_file" (the sed -i.tmp
"...$aidevops_version..." commands) to produce a real newline portably—either
switch to an awk/printf-based edit that inserts the templates_version line after
the "version" line, use a heredoc/temporary file approach (printf/echo with a
real newline to rebuild the file and move it into place), or (preferred) use the
already-checked-for jq to read/modify and write the JSON; ensure you remove the
.tmp cleanup usage accordingly and keep references to $config_file and
$aidevops_version in the updated logic.
🧹 Nitpick comments (2)
aidevops.sh (2)
104-105: Missinglocaldeclaration forchoicevariable.The
choicevariable should be declared aslocalto prevent it from polluting the global scope. As per coding guidelines, use thelocal varpattern.Suggested fix
+ local choice read -r -p "Choice [1]: " choice choice="${choice:-1}"
862-869: Consider warning on unknown arguments.Unknown arguments are silently ignored, which could confuse users who mistype flags (e.g.,
--dryruninstead of--dry-run). A warning would improve UX.Suggested improvement
while [[ $# -gt 0 ]]; do case "$1" in --force|-f) force=true; shift ;; --no-backup) backup=false; shift ;; --dry-run|-n) dry_run=true; shift ;; - *) shift ;; + *) + print_warning "Unknown option: $1" + shift + ;; esac done
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aidevops.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
**/*.sh: Use local var="$1" pattern for shell variable assignments
Use explicit returns in shell scripts
Run ShellCheck on all scripts with zero violations required
Files:
aidevops.sh
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: AGENT.md:0-0
Timestamp: 2025-11-29T04:34:42.033Z
Learning: Maintain all AI assistant instructions, documentation, and operational guidance in AGENTS.md as the single source of truth
📚 Learning: 2025-12-22T01:24:53.937Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: templates/home/AGENTS.md:0-0
Timestamp: 2025-12-22T01:24:53.937Z
Learning: Follow all security protocols and working directory specifications defined in ~/Git/aidevops/AGENTS.md
Applied to files:
aidevops.sh
🧬 Code graph analysis (1)
aidevops.sh (1)
setup.sh (4)
print_warning(28-28)print_info(26-26)print_success(27-27)print_error(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
aidevops.sh (4)
539-542: Clean integration of protected branch check.The early return pattern on failure is appropriate and consistent with the script's error handling style.
1265-1268: LGTM!The main routing correctly shifts before passing arguments to
cmd_upgrade_planning, ensuring flags like--dry-runare properly received.
1202-1215: Help documentation is clear and comprehensive.The upgrade-planning command is well-documented with a helpful example. Good addition of the
upalias for quick access.
1027-1040: Verify task merging with special characters.The awk
-v tasks="$existing_tasks"approach handles most cases well, but tasks containing backslashes, percent signs, or embedded newlines in unusual ways might not merge correctly. Consider testing with tasks containing special shell/awk characters.#!/bin/bash # Check if any existing test files contain tasks with special characters that might break awk -v fd -e md -e txt --exec grep -l '\\' {} \; 2>/dev/null | head -5 || echo "No files with backslashes found"
- Fix awk frontmatter stripping logic (was skipping all content) - Use temp file for awk output to avoid race condition on failure - Replace sed \n with awk for portable newline handling (BSD/macOS) - Only show backup restore hints when --no-backup not used - Add local declaration for choice variable
|
🔍 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: Tue Jan 13 02:34:15 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @aidevops.sh:
- Around line 1035-1047: The merge step currently passes multiline
existing_tasks into awk via -v which causes backslash/escape mangling; instead
export the data into an environment variable and have awk read it via ENVIRON
(e.g. export EXISTING_TASKS="$existing_tasks"; change the awk invocation to
remove -v tasks and inside the script use tasks = ENVIRON["EXISTING_TASKS"] and
print tasks where you previously printed tasks), keep using local
temp_file="${todo_file}.merge", move the temp file into todo_file and call
print_success as before, and finally unset EXISTING_TASKS after the mv to avoid
leaking the variable.
- Around line 1087-1098: The merge logic uses awk with a -v variable ("plans")
which fails on special characters; instead export the plan content to an
environment variable and reference it inside awk via ENVIRON["..."] (e.g., set a
unique env var for existing_plans, change the awk script to insert
ENVIRON["YOUR_VAR"] where it currently uses the -v variable, then unset the env
var after the mv). Update the block handling existing_plans/plans_file to export
the content, use ENVIRON inside the awk script, and clean up the env var to
avoid leaking state while leaving temp_file/mv and print_success behavior
unchanged.
🧹 Nitpick comments (2)
aidevops.sh (2)
1131-1144: Add explicit return for success.Per coding guidelines, shell functions should use explicit returns. The function currently falls through without a return statement on success.
♻️ Add explicit return
[[ "$plans_needs_upgrade" == "true" ]] && echo " mv todo/PLANS.md.bak todo/PLANS.md" fi + + return 0 }
1285-1288: Consider alias clarity:upvsufor different commands.The
upalias forupgrade-planningis close touforupdate. While technically non-conflicting, users might accidentally type one expecting the other. Consider a more distinct alias likeuplanor keeping only the fullupgrade-planningcommand.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
aidevops.sh
🧰 Additional context used
📓 Path-based instructions (1)
**/*.sh
📄 CodeRabbit inference engine (AGENTS.md)
**/*.sh: Use local var="$1" pattern for shell variable assignments
Use explicit returns in shell scripts
Run ShellCheck on all scripts with zero violations required
Files:
aidevops.sh
🧠 Learnings (1)
📚 Learning: 2025-12-22T01:24:53.937Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: templates/home/AGENTS.md:0-0
Timestamp: 2025-12-22T01:24:53.937Z
Learning: Follow all security protocols and working directory specifications defined in ~/Git/aidevops/AGENTS.md
Applied to files:
aidevops.sh
🧬 Code graph analysis (1)
aidevops.sh (1)
setup.sh (4)
print_warning(28-28)print_info(26-26)print_success(27-27)print_error(29-29)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (5)
aidevops.sh (5)
70-155: LGTM! Well-structured protected branch check.The function properly uses local variables, includes explicit returns, and handles all user choices cleanly. The
cd || return 1pattern provides good error handling for directory change failures.
540-543: LGTM! Clean integration of protected branch workflow.The early return pattern properly prevents initialization on protected branches unless the user explicitly chooses to continue.
1020-1028: Good use of temp file pattern to avoid race conditions.The temp file approach with atomic
mvprevents partial writes on failure. The fallback to direct copy when awk fails ensures robustness.
1222-1230: LGTM! Help text properly updated.The new command is documented consistently with existing commands, and the description is clear.
1118-1123: awk\nin sub() replacement works correctly and is the intended portable solution.The current implementation already uses awk specifically because it reliably handles
\nin replacement strings—a workaround for BSD sed's lack of support for\n. The test confirms this approach works correctly and produces the expected output. No change needed.Likely an incorrect or invalid review comment.
| if [[ -n "$existing_tasks" ]]; then | ||
| # Find the Backlog TOON block and insert tasks after its closing --> | ||
| if grep -q "<!--TOON:backlog" "$todo_file"; then | ||
| local temp_file="${todo_file}.merge" | ||
| awk -v tasks="$existing_tasks" ' | ||
| /<!--TOON:backlog/ { in_backlog=1 } | ||
| in_backlog && /^-->$/ { print; print ""; print tasks; in_backlog=0; next } | ||
| { print } | ||
| ' "$todo_file" > "$temp_file" | ||
| mv "$temp_file" "$todo_file" | ||
| print_success "Merged existing tasks into Backlog" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Special characters in task content may be corrupted when merged.
Passing multiline content via awk -v interprets backslash sequences. If tasks contain backslashes (e.g., file paths like C:\Users\... or regex patterns), they'll be mangled. Consider using environment variables with ENVIRON[] instead.
🛠️ Safer approach using ENVIRON
if [[ -n "$existing_tasks" ]]; then
# Find the Backlog TOON block and insert tasks after its closing -->
if grep -q "<!--TOON:backlog" "$todo_file"; then
local temp_file="${todo_file}.merge"
- awk -v tasks="$existing_tasks" '
+ export EXISTING_TASKS="$existing_tasks"
+ awk '
/<!--TOON:backlog/ { in_backlog=1 }
- in_backlog && /^-->$/ { print; print ""; print tasks; in_backlog=0; next }
+ in_backlog && /^-->$/ { print; print ""; print ENVIRON["EXISTING_TASKS"]; in_backlog=0; next }
{ print }
' "$todo_file" > "$temp_file"
+ unset EXISTING_TASKS
mv "$temp_file" "$todo_file"
print_success "Merged existing tasks into Backlog"
fi
fi📝 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.
| if [[ -n "$existing_tasks" ]]; then | |
| # Find the Backlog TOON block and insert tasks after its closing --> | |
| if grep -q "<!--TOON:backlog" "$todo_file"; then | |
| local temp_file="${todo_file}.merge" | |
| awk -v tasks="$existing_tasks" ' | |
| /<!--TOON:backlog/ { in_backlog=1 } | |
| in_backlog && /^-->$/ { print; print ""; print tasks; in_backlog=0; next } | |
| { print } | |
| ' "$todo_file" > "$temp_file" | |
| mv "$temp_file" "$todo_file" | |
| print_success "Merged existing tasks into Backlog" | |
| fi | |
| fi | |
| if [[ -n "$existing_tasks" ]]; then | |
| # Find the Backlog TOON block and insert tasks after its closing --> | |
| if grep -q "<!--TOON:backlog" "$todo_file"; then | |
| local temp_file="${todo_file}.merge" | |
| export EXISTING_TASKS="$existing_tasks" | |
| awk ' | |
| /<!--TOON:backlog/ { in_backlog=1 } | |
| in_backlog && /^-->$/ { print; print ""; print ENVIRON["EXISTING_TASKS"]; in_backlog=0; next } | |
| { print } | |
| ' "$todo_file" > "$temp_file" | |
| unset EXISTING_TASKS | |
| mv "$temp_file" "$todo_file" | |
| print_success "Merged existing tasks into Backlog" | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In @aidevops.sh around lines 1035 - 1047, The merge step currently passes
multiline existing_tasks into awk via -v which causes backslash/escape mangling;
instead export the data into an environment variable and have awk read it via
ENVIRON (e.g. export EXISTING_TASKS="$existing_tasks"; change the awk invocation
to remove -v tasks and inside the script use tasks = ENVIRON["EXISTING_TASKS"]
and print tasks where you previously printed tasks), keep using local
temp_file="${todo_file}.merge", move the temp file into todo_file and call
print_success as before, and finally unset EXISTING_TASKS after the mv to avoid
leaking the variable.
| if [[ -n "$existing_plans" ]]; then | ||
| if grep -q "<!--TOON:active_plans" "$plans_file"; then | ||
| local temp_file="${plans_file}.merge" | ||
| awk -v plans="$existing_plans" ' | ||
| /<!--TOON:active_plans/ { in_active=1 } | ||
| in_active && /^-->$/ { print; print ""; print plans; in_active=0; next } | ||
| { print } | ||
| ' "$plans_file" > "$temp_file" | ||
| mv "$temp_file" "$plans_file" | ||
| print_success "Merged existing plans into Active Plans" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
Same special character issue applies to plans merge.
Apply the same ENVIRON[] fix here for consistency and to prevent data corruption in plan content.
🛠️ Apply ENVIRON pattern
if [[ -n "$existing_plans" ]]; then
if grep -q "<!--TOON:active_plans" "$plans_file"; then
local temp_file="${plans_file}.merge"
- awk -v plans="$existing_plans" '
+ export EXISTING_PLANS="$existing_plans"
+ awk '
/<!--TOON:active_plans/ { in_active=1 }
- in_active && /^-->$/ { print; print ""; print plans; in_active=0; next }
+ in_active && /^-->$/ { print; print ""; print ENVIRON["EXISTING_PLANS"]; in_active=0; next }
{ print }
' "$plans_file" > "$temp_file"
+ unset EXISTING_PLANS
mv "$temp_file" "$plans_file"
print_success "Merged existing plans into Active Plans"
fi
fi📝 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.
| if [[ -n "$existing_plans" ]]; then | |
| if grep -q "<!--TOON:active_plans" "$plans_file"; then | |
| local temp_file="${plans_file}.merge" | |
| awk -v plans="$existing_plans" ' | |
| /<!--TOON:active_plans/ { in_active=1 } | |
| in_active && /^-->$/ { print; print ""; print plans; in_active=0; next } | |
| { print } | |
| ' "$plans_file" > "$temp_file" | |
| mv "$temp_file" "$plans_file" | |
| print_success "Merged existing plans into Active Plans" | |
| fi | |
| fi | |
| if [[ -n "$existing_plans" ]]; then | |
| if grep -q "<!--TOON:active_plans" "$plans_file"; then | |
| local temp_file="${plans_file}.merge" | |
| export EXISTING_PLANS="$existing_plans" | |
| awk ' | |
| /<!--TOON:active_plans/ { in_active=1 } | |
| in_active && /^-->$/ { print; print ""; print ENVIRON["EXISTING_PLANS"]; in_active=0; next } | |
| { print } | |
| ' "$plans_file" > "$temp_file" | |
| unset EXISTING_PLANS | |
| mv "$temp_file" "$plans_file" | |
| print_success "Merged existing plans into Active Plans" | |
| fi | |
| fi |
🤖 Prompt for AI Agents
In @aidevops.sh around lines 1087 - 1098, The merge logic uses awk with a -v
variable ("plans") which fails on special characters; instead export the plan
content to an environment variable and reference it inside awk via
ENVIRON["..."] (e.g., set a unique env var for existing_plans, change the awk
script to insert ENVIRON["YOUR_VAR"] where it currently uses the -v variable,
then unset the env var after the mv). Update the block handling
existing_plans/plans_file to export the content, use ENVIRON inside the awk
script, and clean up the env var to avoid leaking state while leaving
temp_file/mv and print_success behavior unchanged.
1 participant
Summary
aidevops upgrade-planningcommand to upgrade existing TODO.md/PLANS.md files to latest TOON-enhanced templatesProblem
When aidevops templates evolve (e.g., adding TOON markers for machine parsing), existing projects initialized with older templates don't automatically get the improvements. Users had to manually recreate their planning files.
Solution
New CLI command with options:
aidevops upgrade-planning- Interactive upgrade with backupaidevops upgrade-planning --dry-run- Preview changesaidevops upgrade-planning --force- Skip confirmationaidevops upgrade-planning --no-backup- Skip backup creationTesting
Tested with dry-run on awardsapp repo which was initialized with minimal templates:
Checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.