feat: add /session-review and /full-loop commands for comprehensive AI workflow#33
Conversation
- Add session-review.md workflow with objective/workflow/knowledge review - Add session-review-helper.sh script with gather/summary/json commands - Add slash command definition in scripts/commands/ - Update AGENTS.md with command reference and documentation
- Add full-loop-helper.sh orchestrator chaining all phases - Phases: task → preflight → PR → review → postflight → deploy - Auto-detect aidevops repo for setup.sh deployment - Human decision points at merge approval and rollback - Add comprehensive documentation to ralph-loop.md - Add /full-loop slash command
|
Warning Rate limit exceeded@marcusquinn has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 42 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)
WalkthroughThis PR introduces two new slash commands ( Changes
Sequence DiagramssequenceDiagram
participant User
participant full-loop-helper.sh
participant Task Dev Script
participant Preflight Script
participant PR Create Script
participant PR Review Script
participant Postflight Script
participant Deploy Script
User->>full-loop-helper.sh: start [options]
full-loop-helper.sh->>full-loop-helper.sh: Initialize state
full-loop-helper.sh->>Task Dev Script: Run task development phase
Task Dev Script-->>full-loop-helper.sh: <promise>TASK_COMPLETE</promise>
full-loop-helper.sh->>full-loop-helper.sh: Save state
User->>full-loop-helper.sh: resume
full-loop-helper.sh->>full-loop-helper.sh: Load state
full-loop-helper.sh->>Preflight Script: Run preflight checks
Preflight Script-->>full-loop-helper.sh: <promise>PREFLIGHT_PASS</promise>
full-loop-helper.sh->>PR Create Script: Run PR creation
PR Create Script-->>full-loop-helper.sh: <promise>PR_CREATED</promise>
full-loop-helper.sh->>PR Review Script: Run PR review loop
PR Review Script-->>full-loop-helper.sh: <promise>PR_APPROVED</promise>
full-loop-helper.sh->>Postflight Script: Run postflight checks
Postflight Script-->>full-loop-helper.sh: <promise>POSTFLIGHT_PASS</promise>
full-loop-helper.sh->>Deploy Script: Run deployment
Deploy Script-->>full-loop-helper.sh: <promise>DEPLOYED</promise>
full-loop-helper.sh->>full-loop-helper.sh: Clear state
full-loop-helper.sh-->>User: Loop complete
sequenceDiagram
participant User
participant session-review-helper.sh
participant Git
participant gh CLI
participant TODO.md
participant Ralph Loop
User->>session-review-helper.sh: /session-review [--focus]
session-review-helper.sh->>Git: Get branch, commits, status
Git-->>session-review-helper.sh: Branch context
session-review-helper.sh->>TODO.md: Parse objectives
TODO.md-->>session-review-helper.sh: Objective metrics
session-review-helper.sh->>gh CLI: Query open PRs
gh CLI-->>session-review-helper.sh: PR data
session-review-helper.sh->>Ralph Loop: Check loop status
Ralph Loop-->>session-review-helper.sh: Loop state
session-review-helper.sh->>session-review-helper.sh: Compile review sections
session-review-helper.sh-->>User: Objective Completion + Workflow Adherence + Recommendations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes The review spans heterogeneous changes across multiple file types: two substantial Bash helpers with state management and orchestration logic (1,137 combined LOC), five new documentation files with workflow specifications, and updates to core guidance. The helper scripts employ stateful phase progression, error handling with fallbacks, environment validation, and promise-based signaling—all requiring careful logic verification. The workflow documentation must be cross-checked against helper implementations for consistency. Moderate complexity stems from interdependencies and the need to validate both automation logic and documentation accuracy. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 enhances the AI-assisted development framework by integrating two new, powerful slash commands. 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
|
![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 two significant new commands, /session-review and /full-loop, which automate and formalize the development workflow. The changes are well-structured, with new helper scripts, command definitions, and updated documentation. My review focuses on improving the robustness and maintainability of the new shell scripts, particularly around how they parse and handle data. I've provided suggestions to refactor parts of the scripts to be more resilient to changes and easier to read. Overall, this is a great addition that enhances the AI workflow capabilities.
| load_state() { | ||
| if [[ ! -f "$STATE_FILE" ]]; then | ||
| return 1 | ||
| fi | ||
|
|
||
| # Parse YAML frontmatter | ||
| CURRENT_PHASE=$(grep '^phase:' "$STATE_FILE" | cut -d: -f2 | tr -d ' "') | ||
| STARTED_AT=$(grep '^started_at:' "$STATE_FILE" | cut -d: -f2- | tr -d ' "') | ||
| PR_NUMBER=$(grep '^pr_number:' "$STATE_FILE" | cut -d: -f2 | tr -d ' "') | ||
| MAX_TASK_ITERATIONS=$(grep '^max_task_iterations:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| MAX_PREFLIGHT_ITERATIONS=$(grep '^max_preflight_iterations:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| MAX_PR_ITERATIONS=$(grep '^max_pr_iterations:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| SKIP_PREFLIGHT=$(grep '^skip_preflight:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| SKIP_POSTFLIGHT=$(grep '^skip_postflight:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| NO_AUTO_PR=$(grep '^no_auto_pr:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| NO_AUTO_DEPLOY=$(grep '^no_auto_deploy:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
|
|
||
| # Extract prompt (everything after the second ---) | ||
| SAVED_PROMPT=$(sed -n '/^---$/,/^---$/d; p' "$STATE_FILE") | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
The current load_state function uses multiple grep, cut, and tr commands to parse the state file. This is fragile and inefficient as it reads the file multiple times for each key. A more robust and efficient approach is to read the frontmatter block once and parse it with a while read loop. This makes the parsing logic more resilient to formatting changes (like extra whitespace) and easier to maintain.
load_state() {
if [[ ! -f "$STATE_FILE" ]]; then
return 1
fi
# Read frontmatter using a while loop for robustness
while IFS=':' read -r key value; do
# Trim whitespace and quotes
key=$(echo "$key" | tr -d '[:space:]')
value=$(echo "$value" | sed -e 's/^[[:space:]]*//' -e 's/[[:space:]]*$//' -e 's/"//g')
case "$key" in
phase) CURRENT_PHASE="$value" ;;
started_at) STARTED_AT="$value" ;;
pr_number) PR_NUMBER="$value" ;;
max_task_iterations) MAX_TASK_ITERATIONS="$value" ;;
max_preflight_iterations) MAX_PREFLIGHT_ITERATIONS="$value" ;;
max_pr_iterations) MAX_PR_ITERATIONS="$value" ;;
skip_preflight) SKIP_PREFLIGHT="$value" ;;
skip_postflight) SKIP_POSTFLIGHT="$value" ;;
no_auto_pr) NO_AUTO_PR="$value" ;;
no_auto_deploy) NO_AUTO_DEPLOY="$value" ;;
esac
done < <(sed -n '2,/^---$/{/^---$/d;p;}' "$STATE_FILE")
# Extract prompt (everything after the second ---)
SAVED_PROMPT=$(sed -n '/^---$/,/^---$/d; p' "$STATE_FILE")
return 0
}| get_uncommitted_changes() { | ||
| local staged unstaged | ||
| staged=$(git diff --cached --name-only 2>/dev/null | wc -l | tr -d '[:space:]') | ||
| unstaged=$(git diff --name-only 2>/dev/null | wc -l | tr -d '[:space:]') | ||
| echo "staged:${staged:-0},unstaged:${unstaged:-0}" | ||
| } |
There was a problem hiding this comment.
Returning a formatted string like staged:X,unstaged:Y and parsing it in caller functions is fragile. If the format changes, all callers break. A more robust pattern in shell scripting is to echo space-separated values and use read in the caller.
After applying this suggestion, you'll need to update the callers:
In gather_context, replace lines 223-230 with:
local staged unstaged
read -r staged unstaged < <(get_uncommitted_changes)
staged="${staged:-0}"
unstaged="${unstaged:-0}"In output_json, replace lines 249 and 256-257 with:
read -r staged unstaged < <(get_uncommitted_changes)This makes the code more robust and easier to read.
| get_uncommitted_changes() { | |
| local staged unstaged | |
| staged=$(git diff --cached --name-only 2>/dev/null | wc -l | tr -d '[:space:]') | |
| unstaged=$(git diff --name-only 2>/dev/null | wc -l | tr -d '[:space:]') | |
| echo "staged:${staged:-0},unstaged:${unstaged:-0}" | |
| } | |
| get_uncommitted_changes() { | |
| local staged unstaged | |
| staged=$(git diff --cached --name-only 2>/dev/null | wc -l | tr -d '[:space:]') | |
| unstaged=$(git diff --name-only 2>/dev/null | wc -l | tr -d '[:space:]') | |
| echo "${staged:-0} ${unstaged:-0}" | |
| } |
| get_todo_status() { | ||
| local project_root="$1" | ||
| local todo_file="$project_root/TODO.md" | ||
|
|
||
| if [[ ! -f "$todo_file" ]]; then | ||
| echo "no-todo-file" | ||
| return | ||
| fi | ||
|
|
||
| local completed incomplete in_progress | ||
| completed=$(grep -cE '^\s*- \[x\]' "$todo_file" 2>/dev/null) || completed=0 | ||
| incomplete=$(grep -cE '^\s*- \[ \]' "$todo_file" 2>/dev/null) || incomplete=0 | ||
| in_progress=$(grep -cE '^\s*- \[>\]' "$todo_file" 2>/dev/null) || in_progress=0 | ||
|
|
||
| echo "completed:$completed,incomplete:$incomplete,in_progress:$in_progress" | ||
| } |
There was a problem hiding this comment.
Similar to get_uncommitted_changes, this function returns a formatted string that is parsed by callers, which is a fragile pattern. It's more robust to return space-separated values and use read in the callers.
After applying this suggestion, you will need to update the call sites in gather_context and output_json. For example, you would read the values like this:
local completed incomplete in_progress
read -r completed incomplete in_progress < <(get_todo_status "$project_root")And you would need to adjust how the status is printed, for example on line 176:
echo "TODO status: completed:$completed,incomplete:$incomplete,in_progress:$in_progress"| get_todo_status() { | |
| local project_root="$1" | |
| local todo_file="$project_root/TODO.md" | |
| if [[ ! -f "$todo_file" ]]; then | |
| echo "no-todo-file" | |
| return | |
| fi | |
| local completed incomplete in_progress | |
| completed=$(grep -cE '^\s*- \[x\]' "$todo_file" 2>/dev/null) || completed=0 | |
| incomplete=$(grep -cE '^\s*- \[ \]' "$todo_file" 2>/dev/null) || incomplete=0 | |
| in_progress=$(grep -cE '^\s*- \[>\]' "$todo_file" 2>/dev/null) || in_progress=0 | |
| echo "completed:$completed,incomplete:$incomplete,in_progress:$in_progress" | |
| } | |
| get_todo_status() { | |
| local project_root="$1" | |
| local todo_file="$project_root/TODO.md" | |
| if [[ ! -f "$todo_file" ]]; then | |
| echo "0 0 0" | |
| return | |
| fi | |
| local completed incomplete in_progress | |
| completed=$(grep -cE '^\s*- \[x\]' "$todo_file" 2>/dev/null) || completed=0 | |
| incomplete=$(grep -cE '^\s*- \[ \]' "$todo_file" 2>/dev/null) || incomplete=0 | |
| in_progress=$(grep -cE '^\s*- \[>\]' "$todo_file" 2>/dev/null) || in_progress=0 | |
| echo "$completed $incomplete $in_progress" | |
| } |
🤖 Augment PR SummarySummary: Adds two new slash commands to improve AI-assisted development workflows: session auditing and an end-to-end development loop. Changes:
full-loop-helper.sh persists resumable state in .claude/full-loop.local.md and conditionally runs ./setup.sh when the repo is detected as aidevops.
🤖 Was this summary useful? React with 👍 or 👎 |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
.agent/scripts/full-loop-helper.sh
Outdated
| return 0 | ||
| fi | ||
|
|
||
| echo "<promise>PR_MERGED</promise>" |
There was a problem hiding this comment.
run_pr_review_phase always emits <promise>PR_MERGED</promise> even though quality-loop-helper.sh pr-review can return success on approval (not merge), and the fallback path returns 0 while telling the user to merge manually. That can cause resume to advance into postflight/deploy even when nothing has been merged yet.
🤖 Was this useful? React with 👍 or 👎
.agent/scripts/full-loop-helper.sh
Outdated
| fi | ||
| fi | ||
|
|
||
| echo "<promise>PREFLIGHT_PASS</promise>" |
There was a problem hiding this comment.
run_preflight_phase emits <promise>PREFLIGHT_PASS</promise> even when it prints "No preflight script found, skipping" (and this same pattern exists in run_postflight_phase with RELEASE_HEALTHY). This can mislead any automation/agent logic that treats the promise output as an actual “checks ran and passed” signal.
🤖 Was this useful? React with 👍 or 👎
| local todo_status | ||
| todo_status=$(get_todo_status "$project_root") | ||
| local incomplete | ||
| incomplete=$(echo "$todo_status" | grep -oE 'incomplete:[0-9]+' | cut -d: -f2) |
There was a problem hiding this comment.
If TODO.md is missing, get_todo_status returns no-todo-file, and with set -euo pipefail the grep -oE 'incomplete:...' pipeline here will cause gather to exit early. That means /session-review can fail outright in repos that don’t use TODO tracking.
🤖 Was this useful? React with 👍 or 👎
|
|
||
| # Check 2: Recent commits have good messages | ||
| local short_messages | ||
| short_messages=$(git log --oneline -5 2>/dev/null | awk 'length($0) < 15' | wc -l | tr -d ' ') |
There was a problem hiding this comment.
In check_workflow_adherence, the git log | awk | wc pipeline will terminate the script under set -euo pipefail when run outside a git worktree (since git log returns non-zero). If this helper is intended to work in directories found via TODO.md without .git, it likely needs to treat "not a git repo" as a soft state rather than a hard failure.
🤖 Was this useful? React with 👍 or 👎
- Handle missing TODO.md gracefully in session-review-helper.sh - Handle non-git repos gracefully in check_workflow_adherence - Don't emit false promises in full-loop-helper.sh phases - Use distinct promises: PREFLIGHT_SKIPPED, POSTFLIGHT_SKIPPED, PR_APPROVED, PR_WAITING - Verify PR is actually merged before emitting PR_MERGED promise
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In @.agent/scripts/full-loop-helper.sh:
- Around line 322-360: The PR review currently emits
"<promise>PR_APPROVED</promise>" even when the PR isn't merged; change
run_pr_review_phase so it only signals progression when the PR state is MERGED
and otherwise emits "<promise>PR_WAITING</promise>" and returns a non-zero
status to prevent advancing to postflight/deploy; update the branch of
run_pr_review_phase that currently prints PR_APPROVED to instead print
PR_WAITING and return 1, and make the same change in the other PR-review block
referenced (lines ~557-565) so both paths consistently gate cmd_resume until the
PR is actually merged.
- Around line 238-245: The run_preflight_phase function currently echoes
"<promise>PREFLIGHT_PASS</promise>" when SKIP_PREFLIGHT is true; change this to
not emit PREFLIGHT_PASS (e.g., echo "<promise>PREFLIGHT_SKIPPED</promise>" or
remove the echo entirely) so the --skip-preflight path does not signal a passing
preflight; update the SKIP_PREFLIGHT check block (the if branch in
run_preflight_phase) to reflect the new token or omit the promise and still
return 0.
- Around line 433-473: Option parsing uses $2 without validation which will
break under set -u and when flags are missing values; update the branches that
accept arguments (the --max-task-iterations, --max-preflight-iterations,
--max-pr-iterations cases) to first check that a next argument exists (e.g. [[
-z "${2:-}" || "${2}" == --* ]] → print_error and return 1), then validate it is
numeric (use a regex like [[ "$2" =~ ^[0-9]+$ ]] else error), only then assign
MAX_TASK_ITERATIONS/ MAX_PREFLIGHT_ITERATIONS/ MAX_PR_ITERATIONS and shift 2;
keep boolean flags (--skip-preflight, --skip-postflight, --no-auto-pr,
--no-auto-deploy, --dry-run) unchanged.
- Around line 628-654: The cmd_complete function currently prints definitive
status lines (Passed/Merged/Healthy/Complete) even when phases were skipped or
PR_NUMBER is unknown; modify cmd_complete to load_state (using load_state) and
conditionally print each summary line only if corresponding state flags exist or
checks succeed (e.g., only print "Preflight: Passed" if a saved preflight status
is present, only print "PR: #... merged" when PR_NUMBER is set and verify merge
state if possible, only print "Postflight: Healthy" if postflight status
indicates healthy, and only print "Deploy: Complete" when is_aidevops_repo
returns true and a deploy flag exists); ensure clear_state is still called at
the end and keep the final promise tag but reflect actual known states rather
than hardcoded successes.
- Around line 116-166: The state file uses '---' to separate YAML frontmatter
from the prompt, so load_state's sed extraction breaks if the prompt itself
contains '---'; update save_state to write a unique sentinel marker (e.g.,
__PROMPT_START__ on its own line) before the prompt in save_state, and update
load_state to extract the prompt by reading everything after that sentinel into
SAVED_PROMPT (for example use sed or awk to find the sentinel line and capture
all following lines), keeping references to save_state, load_state, STATE_FILE
and SAVED_PROMPT so the change is localized and robust.
- Around line 181-201: The function is_aidevops_repo currently proceeds when git
rev-parse fails, causing an empty repo_root and accidental checks against the
filesystem root; update is_aidevops_repo so that after repo_root=$(git rev-parse
--show-toplevel 2>/dev/null || echo "") you detect an empty repo_root (if [[ -z
"$repo_root" ]]) and return 1 immediately, preventing subsequent checks against
"/.aidevops-repo" or "/setup.sh"; keep the existing marker checks
(.aidevops-repo, setup.sh and grep) unchanged otherwise.
In @.agent/workflows/session-review.md:
- Around line 113-134: The check using git diff --name-only HEAD~5 can fail in
shallow or short-history repos; update the workflow to detect commit count and
fall back to a safe reference (e.g., use HEAD~1 or git merge-base
--fork-point/merge-base with origin/main) before calling git diff; specifically
modify the step that runs "git diff --name-only HEAD~5" to first compute a valid
base (check commit count or compute merge-base) and then call git diff with that
computed base so the TODO.md check never errors on shallow histories.
🧹 Nitpick comments (2)
.agent/scripts/session-review-helper.sh (1)
138-138: Consider relaxing the short commit message threshold.The current threshold of 15 characters would flag many valid conventional commit messages like "feat: add tests" (16 chars) or "fix: bug" (9 chars). Consider raising this to 20-30 characters or checking for conventional commit prefixes before flagging.
♻️ Suggested adjustment
- short_messages=$(git log --oneline -5 2>/dev/null | awk 'length($0) < 15' | wc -l | tr -d ' ' || echo "0") + short_messages=$(git log --oneline -5 2>/dev/null | awk 'length($0) < 20' | wc -l | tr -d ' ' || echo "0")Or better yet, exclude conventional commit messages:
- short_messages=$(git log --oneline -5 2>/dev/null | awk 'length($0) < 15' | wc -l | tr -d ' ' || echo "0") + # Flag commits under 20 chars that don't start with conventional prefixes + short_messages=$(git log --oneline -5 2>/dev/null | awk '!/^[a-f0-9]+ (feat|fix|docs|style|refactor|test|chore|build|ci|perf|revert):/ && length($0) < 20' | wc -l | tr -d ' ' || echo "0").agent/workflows/session-review.md (1)
62-78: Tighten the suggested command snippets (less brittle, fewer footguns).
- Prefer
head -n 10 .claude/ralph-loop.local.mdovercat ... | head -10.- Consider noting that
gh ...requires auth/installed, and examples should degrade cleanly whenghisn’t available (similar to how you handle missingTODO.md).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.agent/AGENTS.md.agent/scripts/commands/full-loop.md.agent/scripts/commands/session-review.md.agent/scripts/full-loop-helper.sh.agent/scripts/session-review-helper.sh.agent/workflows/ralph-loop.md.agent/workflows/session-review.md
🧰 Additional context used
📓 Path-based instructions (1)
.agent/scripts/*.sh
⚙️ CodeRabbit configuration file
.agent/scripts/*.sh: Automation scripts - focus on:
- Reliability and robustness
- Clear logging and feedback
- Proper exit codes
- Error recovery mechanisms
Files:
.agent/scripts/session-review-helper.sh.agent/scripts/full-loop-helper.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/scripts/commands/session-review.md.agent/AGENTS.md.agent/scripts/commands/full-loop.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/scripts/commands/session-review.md.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/scripts/commands/session-review.md.agent/AGENTS.md.agent/scripts/commands/full-loop.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/scripts/commands/session-review.md.agent/AGENTS.md.agent/scripts/commands/full-loop.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: 2026-01-06T15:57:56.007Z
Learnt from: CR
Repo: marcusquinn/aidevops PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-06T15:57:56.007Z
Learning: Applies to **/.agent/scripts/linters-local.sh : Run quality checks before committing using .agent/scripts/linters-local.sh
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: Follow all security protocols and working directory specifications defined in ~/Git/aidevops/AGENTS.md
Applied to files:
.agent/scripts/session-review-helper.sh
🧬 Code graph analysis (2)
.agent/scripts/session-review-helper.sh (2)
.agent/scripts/system-cleanup.sh (1)
log(52-76).agent/scripts/full-loop-helper.sh (2)
show_help(656-710)main(716-744)
.agent/scripts/full-loop-helper.sh (1)
.opencode/tool/mcp-integrations.ts (1)
setup(3-13)
🪛 LanguageTool
.agent/scripts/commands/session-review.md
[grammar] ~38-~38: Ensure spelling is correct
Context: ...tep 3: Check Workflow Adherence Verify aidevops best practices: | Practice | Check | R...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
.agent/workflows/session-review.md
[grammar] ~111-~111: Ensure spelling is correct
Context: ...tep 3: Workflow Adherence Check Verify aidevops best practices were followed: | Practi...
(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)
.agent/scripts/session-review-helper.sh (2)
85-98: Verify Ralph loop state file parsing robustness.The iteration and max_iterations extraction uses
grep | cutwhich could fail silently if the file format doesn't match exactly (e.g., extra spaces, different delimiters).Run the following script to verify the parsing handles various edge cases:
#!/bin/bash # Test Ralph state file parsing with edge cases test_dir=$(mktemp -d) cd "$test_dir" || exit 1 # Create test state files mkdir -p .claude # Test 1: Normal format cat > .claude/ralph-loop.local.md << 'EOF' iteration:5 max_iterations:50 EOF # Test 2: Extra spaces cat > .claude/ralph-loop-spaces.local.md << 'EOF' iteration: 5 max_iterations: 50 EOF # Test 3: Missing max_iterations cat > .claude/ralph-loop-partial.local.md << 'EOF' iteration:5 EOF # Source the script functions and test echo "Testing normal format..." source ~/.aidevops/agents/scripts/session-review-helper.sh result=$(get_ralph_status "$test_dir") echo "Result: $result" echo "Testing with spaces..." # Manually test the grep/cut pattern iteration=$(grep '^iteration:' .claude/ralph-loop-spaces.local.md | cut -d: -f2 | tr -d ' ') max_iter=$(grep '^max_iterations:' .claude/ralph-loop-spaces.local.md | cut -d: -f2 | tr -d ' ') echo "iteration=$iteration, max_iter=$max_iter" # Cleanup cd / && rm -rf "$test_dir"
1-391: Overall script quality is excellent with strong error handling.The script demonstrates solid DevOps automation practices:
- ✅ Strict error handling with
set -euo pipefail- ✅ Comprehensive error fallbacks for all external commands
- ✅ Clear color-coded output for user feedback
- ✅ Proper readonly constants
- ✅ Defensive coding with
${var:-default}patterns- ✅ Multiple output formats (human, JSON, summary)
- ✅ Non-zero exit only on invalid arguments
The implementation aligns well with the reliability and robustness requirements from coding guidelines.
.agent/scripts/commands/full-loop.md (1)
1-142: Documentation is comprehensive and well-structured.The full-loop command documentation provides clear guidance on:
- Prerequisites and validation steps
- Complete workflow from task to deployment
- Human decision points for control
- Options and examples for various scenarios
- Cross-references to related workflows
The documentation aligns well with the PR objectives and the full-loop-helper.sh script implementation referenced in the relevant code snippets.
.agent/AGENTS.md (3)
114-114: Slash commands list properly updated with new commands.The addition of
/session-reviewand/full-loopto the slash commands list maintains consistency with existing entries and aligns with the PR's introduction of these new workflows.
339-339: Workflows table accurately updated.The addition of
session-reviewto the workflows folder description maintains alphabetical order and consistency with the existing format.
444-445: Key scripts table properly extended.The new helper scripts are documented with clear purpose descriptions that match the script functionality documented elsewhere in the PR.
.agent/scripts/commands/session-review.md (1)
1-154: Session review documentation is thorough and actionable.The documentation provides:
- Clear 5-step review process
- Objective completion scoring criteria
- Comprehensive workflow adherence checklist
- Knowledge capture guidance with specific storage locations
- Session health assessment decision criteria
- Detailed output format template
- Integration points with related workflows
The structure enables consistent and comprehensive session reviews.
.agent/workflows/ralph-loop.md (1)
391-515: Full Development Loop section is comprehensive and well-integrated.The new section provides:
- Clear introduction and quick start commands
- Visual ASCII diagram of the 6-phase workflow
- Detailed phase table with scripts, promises, and triggers
- Explicit human decision points vs. AI autonomy
- Comprehensive options documentation
- aidevops-specific behavior explanation
- State management details for resumption
The addition integrates seamlessly with the existing Ralph loop documentation and maintains consistent formatting and depth.
| save_state() { | ||
| local phase="$1" | ||
| local prompt="$2" | ||
| local pr_number="${3:-}" | ||
| local started_at="${4:-$(date -u '+%Y-%m-%dT%H:%M:%SZ')}" | ||
|
|
||
| init_state_dir | ||
|
|
||
| cat > "$STATE_FILE" << EOF | ||
| --- | ||
| active: true | ||
| phase: ${phase} | ||
| started_at: "${started_at}" | ||
| updated_at: "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" | ||
| pr_number: "${pr_number}" | ||
| max_task_iterations: ${MAX_TASK_ITERATIONS:-$DEFAULT_MAX_TASK_ITERATIONS} | ||
| max_preflight_iterations: ${MAX_PREFLIGHT_ITERATIONS:-$DEFAULT_MAX_PREFLIGHT_ITERATIONS} | ||
| max_pr_iterations: ${MAX_PR_ITERATIONS:-$DEFAULT_MAX_PR_ITERATIONS} | ||
| skip_preflight: ${SKIP_PREFLIGHT:-false} | ||
| skip_postflight: ${SKIP_POSTFLIGHT:-false} | ||
| no_auto_pr: ${NO_AUTO_PR:-false} | ||
| no_auto_deploy: ${NO_AUTO_DEPLOY:-false} | ||
| --- | ||
|
|
||
| ${prompt} | ||
| EOF | ||
| return 0 | ||
| } | ||
|
|
||
| load_state() { | ||
| if [[ ! -f "$STATE_FILE" ]]; then | ||
| return 1 | ||
| fi | ||
|
|
||
| # Parse YAML frontmatter | ||
| CURRENT_PHASE=$(grep '^phase:' "$STATE_FILE" | cut -d: -f2 | tr -d ' "') | ||
| STARTED_AT=$(grep '^started_at:' "$STATE_FILE" | cut -d: -f2- | tr -d ' "') | ||
| PR_NUMBER=$(grep '^pr_number:' "$STATE_FILE" | cut -d: -f2 | tr -d ' "') | ||
| MAX_TASK_ITERATIONS=$(grep '^max_task_iterations:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| MAX_PREFLIGHT_ITERATIONS=$(grep '^max_preflight_iterations:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| MAX_PR_ITERATIONS=$(grep '^max_pr_iterations:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| SKIP_PREFLIGHT=$(grep '^skip_preflight:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| SKIP_POSTFLIGHT=$(grep '^skip_postflight:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| NO_AUTO_PR=$(grep '^no_auto_pr:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
| NO_AUTO_DEPLOY=$(grep '^no_auto_deploy:' "$STATE_FILE" | cut -d: -f2 | tr -d ' ') | ||
|
|
||
| # Extract prompt (everything after the second ---) | ||
| SAVED_PROMPT=$(sed -n '/^---$/,/^---$/d; p' "$STATE_FILE") | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
State persistence is fragile if the prompt contains --- (breaks load_state extraction).
Proposed fix (use explicit prompt sentinels instead of `---`-based extraction)
save_state() {
local phase="$1"
local prompt="$2"
@@
cat > "$STATE_FILE" << EOF
---
active: true
phase: ${phase}
started_at: "${started_at}"
updated_at: "$(date -u '+%Y-%m-%dT%H:%M:%SZ')"
pr_number: "${pr_number}"
max_task_iterations: ${MAX_TASK_ITERATIONS:-$DEFAULT_MAX_TASK_ITERATIONS}
max_preflight_iterations: ${MAX_PREFLIGHT_ITERATIONS:-$DEFAULT_MAX_PREFLIGHT_ITERATIONS}
max_pr_iterations: ${MAX_PR_ITERATIONS:-$DEFAULT_MAX_PR_ITERATIONS}
skip_preflight: ${SKIP_PREFLIGHT:-false}
skip_postflight: ${SKIP_POSTFLIGHT:-false}
no_auto_pr: ${NO_AUTO_PR:-false}
no_auto_deploy: ${NO_AUTO_DEPLOY:-false}
---
-
-${prompt}
+# PROMPT_START
+${prompt}
+# PROMPT_END
EOF
return 0
}
@@
load_state() {
@@
- # Extract prompt (everything after the second ---)
- SAVED_PROMPT=$(sed -n '/^---$/,/^---$/d; p' "$STATE_FILE")
+ # Extract prompt (sentinel-delimited)
+ SAVED_PROMPT=$(
+ awk 'f{ if($0=="# PROMPT_END"){exit} print }
+ $0=="# PROMPT_START"{f=1}' "$STATE_FILE"
+ )
return 0
}🤖 Prompt for AI Agents
In @.agent/scripts/full-loop-helper.sh around lines 116 - 166, The state file
uses '---' to separate YAML frontmatter from the prompt, so load_state's sed
extraction breaks if the prompt itself contains '---'; update save_state to
write a unique sentinel marker (e.g., __PROMPT_START__ on its own line) before
the prompt in save_state, and update load_state to extract the prompt by reading
everything after that sentinel into SAVED_PROMPT (for example use sed or awk to
find the sentinel line and capture all following lines), keeping references to
save_state, load_state, STATE_FILE and SAVED_PROMPT so the change is localized
and robust.
| is_aidevops_repo() { | ||
| local repo_root | ||
| repo_root=$(git rev-parse --show-toplevel 2>/dev/null || echo "") | ||
|
|
||
| # Check if repo name contains aidevops | ||
| if [[ "$repo_root" == *"/aidevops"* ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| # Check for marker file | ||
| if [[ -f "$repo_root/.aidevops-repo" ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| # Check if setup.sh exists and contains aidevops marker | ||
| if [[ -f "$repo_root/setup.sh" ]] && grep -q "aidevops" "$repo_root/setup.sh" 2>/dev/null; then | ||
| return 0 | ||
| fi | ||
|
|
||
| return 1 | ||
| } |
There was a problem hiding this comment.
Fix is_aidevops_repo behavior outside git repos (empty repo_root can check /.aidevops-repo / /setup.sh).
Proposed fix
is_aidevops_repo() {
local repo_root
- repo_root=$(git rev-parse --show-toplevel 2>/dev/null || echo "")
+ repo_root=$(git rev-parse --show-toplevel 2>/dev/null || echo "")
+ [[ -n "$repo_root" ]] || return 1
# Check if repo name contains aidevops
if [[ "$repo_root" == *"/aidevops"* ]]; then
return 0
fi🤖 Prompt for AI Agents
In @.agent/scripts/full-loop-helper.sh around lines 181 - 201, The function
is_aidevops_repo currently proceeds when git rev-parse fails, causing an empty
repo_root and accidental checks against the filesystem root; update
is_aidevops_repo so that after repo_root=$(git rev-parse --show-toplevel
2>/dev/null || echo "") you detect an empty repo_root (if [[ -z "$repo_root" ]])
and return 1 immediately, preventing subsequent checks against "/.aidevops-repo"
or "/setup.sh"; keep the existing marker checks (.aidevops-repo, setup.sh and
grep) unchanged otherwise.
| run_preflight_phase() { | ||
| print_phase "Preflight" "Running quality checks before commit" | ||
|
|
||
| if [[ "${SKIP_PREFLIGHT:-false}" == "true" ]]; then | ||
| print_warning "Preflight skipped by user request" | ||
| echo "<promise>PREFLIGHT_PASS</promise>" | ||
| return 0 | ||
| fi |
There was a problem hiding this comment.
--skip-preflight must not emit PREFLIGHT_PASS.
Proposed fix
if [[ "${SKIP_PREFLIGHT:-false}" == "true" ]]; then
print_warning "Preflight skipped by user request"
- echo "<promise>PREFLIGHT_PASS</promise>"
+ echo "<promise>PREFLIGHT_SKIPPED</promise>"
return 0
fi🤖 Prompt for AI Agents
In @.agent/scripts/full-loop-helper.sh around lines 238 - 245, The
run_preflight_phase function currently echoes
"<promise>PREFLIGHT_PASS</promise>" when SKIP_PREFLIGHT is true; change this to
not emit PREFLIGHT_PASS (e.g., echo "<promise>PREFLIGHT_SKIPPED</promise>" or
remove the echo entirely) so the --skip-preflight path does not signal a passing
preflight; update the SKIP_PREFLIGHT check block (the if branch in
run_preflight_phase) to reflect the new token or omit the promise and still
return 0.
| run_pr_review_phase() { | ||
| print_phase "PR Review" "Monitoring PR for approval and CI checks" | ||
|
|
||
| local pr_number="${PR_NUMBER:-}" | ||
|
|
||
| if [[ -z "$pr_number" ]]; then | ||
| # Try to get PR number from current branch | ||
| pr_number=$(gh pr view --json number --jq '.number' 2>/dev/null || echo "") | ||
| fi | ||
|
|
||
| if [[ -z "$pr_number" ]]; then | ||
| print_error "No PR number found. Create PR first." | ||
| return 1 | ||
| fi | ||
|
|
||
| print_info "Monitoring PR #$pr_number..." | ||
|
|
||
| # Run quality loop for PR review | ||
| if [[ -x "$SCRIPT_DIR/quality-loop-helper.sh" ]]; then | ||
| "$SCRIPT_DIR/quality-loop-helper.sh" pr-review --pr "$pr_number" --wait-for-ci --max-iterations "${MAX_PR_ITERATIONS:-$DEFAULT_MAX_PR_ITERATIONS}" | ||
|
|
||
| # Verify PR was actually merged before emitting promise | ||
| local pr_state | ||
| pr_state=$(gh pr view "$pr_number" --json state --jq '.state' 2>/dev/null || echo "UNKNOWN") | ||
| if [[ "$pr_state" == "MERGED" ]]; then | ||
| echo "<promise>PR_MERGED</promise>" | ||
| else | ||
| print_warning "PR #$pr_number is $pr_state (not merged yet)" | ||
| print_info "Merge PR manually, then run: full-loop-helper.sh resume" | ||
| echo "<promise>PR_APPROVED</promise>" | ||
| fi | ||
| else | ||
| print_warning "quality-loop-helper.sh not found, waiting for manual merge" | ||
| print_info "Merge PR manually, then run: full-loop-helper.sh resume" | ||
| echo "<promise>PR_WAITING</promise>" | ||
| fi | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
Critical: don’t advance to postflight/deploy unless the PR is actually merged.
Proposed fix (make PR review return “waiting”, and gate `cmd_resume` progression)
run_pr_review_phase() {
@@
- local pr_number="${PR_NUMBER:-}"
+ local pr_number="${PR_NUMBER:-}"
@@
if [[ -z "$pr_number" ]]; then
print_error "No PR number found. Create PR first."
return 1
fi
+ PR_NUMBER="$pr_number"
@@
- if [[ "$pr_state" == "MERGED" ]]; then
- echo "<promise>PR_MERGED</promise>"
- else
+ if [[ "$pr_state" == "MERGED" ]]; then
+ echo "<promise>PR_MERGED</promise>"
+ return 0
+ else
print_warning "PR #$pr_number is $pr_state (not merged yet)"
print_info "Merge PR manually, then run: full-loop-helper.sh resume"
echo "<promise>PR_APPROVED</promise>"
+ return 2
fi
else
print_warning "quality-loop-helper.sh not found, waiting for manual merge"
print_info "Merge PR manually, then run: full-loop-helper.sh resume"
echo "<promise>PR_WAITING</promise>"
+ return 2
fi
-
- return 0
}
@@
"$PHASE_PR_REVIEW")
- run_pr_review_phase
+ local pr_rc=0
+ run_pr_review_phase || pr_rc=$?
+ # Stay in PR_REVIEW until merged (rc=2).
+ if [[ $pr_rc -eq 2 ]]; then
+ save_state "$PHASE_PR_REVIEW" "$SAVED_PROMPT" "$PR_NUMBER" "$STARTED_AT"
+ return 0
+ elif [[ $pr_rc -ne 0 ]]; then
+ return "$pr_rc"
+ fi
save_state "$PHASE_POSTFLIGHT" "$SAVED_PROMPT" "$PR_NUMBER" "$STARTED_AT"
run_postflight_phase
save_state "$PHASE_DEPLOY" "$SAVED_PROMPT" "$PR_NUMBER" "$STARTED_AT"
run_deploy_phase
save_state "$PHASE_COMPLETE" "$SAVED_PROMPT" "$PR_NUMBER" "$STARTED_AT"
cmd_complete
;;Also applies to: 557-565
🤖 Prompt for AI Agents
In @.agent/scripts/full-loop-helper.sh around lines 322 - 360, The PR review
currently emits "<promise>PR_APPROVED</promise>" even when the PR isn't merged;
change run_pr_review_phase so it only signals progression when the PR state is
MERGED and otherwise emits "<promise>PR_WAITING</promise>" and returns a
non-zero status to prevent advancing to postflight/deploy; update the branch of
run_pr_review_phase that currently prints PR_APPROVED to instead print
PR_WAITING and return 1, and make the same change in the other PR-review block
referenced (lines ~557-565) so both paths consistently gate cmd_resume until the
PR is actually merged.
| # Parse options | ||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --max-task-iterations) | ||
| MAX_TASK_ITERATIONS="$2" | ||
| shift 2 | ||
| ;; | ||
| --max-preflight-iterations) | ||
| MAX_PREFLIGHT_ITERATIONS="$2" | ||
| shift 2 | ||
| ;; | ||
| --max-pr-iterations) | ||
| MAX_PR_ITERATIONS="$2" | ||
| shift 2 | ||
| ;; | ||
| --skip-preflight) | ||
| SKIP_PREFLIGHT=true | ||
| shift | ||
| ;; | ||
| --skip-postflight) | ||
| SKIP_POSTFLIGHT=true | ||
| shift | ||
| ;; | ||
| --no-auto-pr) | ||
| NO_AUTO_PR=true | ||
| shift | ||
| ;; | ||
| --no-auto-deploy) | ||
| NO_AUTO_DEPLOY=true | ||
| shift | ||
| ;; | ||
| --dry-run) | ||
| DRY_RUN=true | ||
| shift | ||
| ;; | ||
| *) | ||
| print_error "Unknown option: $1" | ||
| return 1 | ||
| ;; | ||
| esac | ||
| done |
There was a problem hiding this comment.
Harden option parsing: missing values will crash under set -u.
Proposed fix (validate presence + basic numeric check)
while [[ $# -gt 0 ]]; do
case "$1" in
--max-task-iterations)
+ [[ $# -ge 2 ]] || { print_error "Missing value for --max-task-iterations"; return 1; }
+ [[ "$2" =~ ^[0-9]+$ ]] || { print_error "Invalid --max-task-iterations: $2"; return 1; }
MAX_TASK_ITERATIONS="$2"
shift 2
;;
--max-preflight-iterations)
+ [[ $# -ge 2 ]] || { print_error "Missing value for --max-preflight-iterations"; return 1; }
+ [[ "$2" =~ ^[0-9]+$ ]] || { print_error "Invalid --max-preflight-iterations: $2"; return 1; }
MAX_PREFLIGHT_ITERATIONS="$2"
shift 2
;;
--max-pr-iterations)
+ [[ $# -ge 2 ]] || { print_error "Missing value for --max-pr-iterations"; return 1; }
+ [[ "$2" =~ ^[0-9]+$ ]] || { print_error "Invalid --max-pr-iterations: $2"; return 1; }
MAX_PR_ITERATIONS="$2"
shift 2
;;📝 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.
| # Parse options | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --max-task-iterations) | |
| MAX_TASK_ITERATIONS="$2" | |
| shift 2 | |
| ;; | |
| --max-preflight-iterations) | |
| MAX_PREFLIGHT_ITERATIONS="$2" | |
| shift 2 | |
| ;; | |
| --max-pr-iterations) | |
| MAX_PR_ITERATIONS="$2" | |
| shift 2 | |
| ;; | |
| --skip-preflight) | |
| SKIP_PREFLIGHT=true | |
| shift | |
| ;; | |
| --skip-postflight) | |
| SKIP_POSTFLIGHT=true | |
| shift | |
| ;; | |
| --no-auto-pr) | |
| NO_AUTO_PR=true | |
| shift | |
| ;; | |
| --no-auto-deploy) | |
| NO_AUTO_DEPLOY=true | |
| shift | |
| ;; | |
| --dry-run) | |
| DRY_RUN=true | |
| shift | |
| ;; | |
| *) | |
| print_error "Unknown option: $1" | |
| return 1 | |
| ;; | |
| esac | |
| done | |
| # Parse options | |
| while [[ $# -gt 0 ]]; do | |
| case "$1" in | |
| --max-task-iterations) | |
| [[ $# -ge 2 ]] || { print_error "Missing value for --max-task-iterations"; return 1; } | |
| [[ "$2" =~ ^[0-9]+$ ]] || { print_error "Invalid --max-task-iterations: $2"; return 1; } | |
| MAX_TASK_ITERATIONS="$2" | |
| shift 2 | |
| ;; | |
| --max-preflight-iterations) | |
| [[ $# -ge 2 ]] || { print_error "Missing value for --max-preflight-iterations"; return 1; } | |
| [[ "$2" =~ ^[0-9]+$ ]] || { print_error "Invalid --max-preflight-iterations: $2"; return 1; } | |
| MAX_PREFLIGHT_ITERATIONS="$2" | |
| shift 2 | |
| ;; | |
| --max-pr-iterations) | |
| [[ $# -ge 2 ]] || { print_error "Missing value for --max-pr-iterations"; return 1; } | |
| [[ "$2" =~ ^[0-9]+$ ]] || { print_error "Invalid --max-pr-iterations: $2"; return 1; } | |
| MAX_PR_ITERATIONS="$2" | |
| shift 2 | |
| ;; | |
| --skip-preflight) | |
| SKIP_PREFLIGHT=true | |
| shift | |
| ;; | |
| --skip-postflight) | |
| SKIP_POSTFLIGHT=true | |
| shift | |
| ;; | |
| --no-auto-pr) | |
| NO_AUTO_PR=true | |
| shift | |
| ;; | |
| --no-auto-deploy) | |
| NO_AUTO_DEPLOY=true | |
| shift | |
| ;; | |
| --dry-run) | |
| DRY_RUN=true | |
| shift | |
| ;; | |
| *) | |
| print_error "Unknown option: $1" | |
| return 1 | |
| ;; | |
| esac | |
| done |
🤖 Prompt for AI Agents
In @.agent/scripts/full-loop-helper.sh around lines 433 - 473, Option parsing
uses $2 without validation which will break under set -u and when flags are
missing values; update the branches that accept arguments (the
--max-task-iterations, --max-preflight-iterations, --max-pr-iterations cases) to
first check that a next argument exists (e.g. [[ -z "${2:-}" || "${2}" == --* ]]
→ print_error and return 1), then validate it is numeric (use a regex like [[
"$2" =~ ^[0-9]+$ ]] else error), only then assign MAX_TASK_ITERATIONS/
MAX_PREFLIGHT_ITERATIONS/ MAX_PR_ITERATIONS and shift 2; keep boolean flags
(--skip-preflight, --skip-postflight, --no-auto-pr, --no-auto-deploy, --dry-run)
unchanged.
| cmd_complete() { | ||
| echo "" | ||
| echo -e "${BOLD}${GREEN}╔════════════════════════════════════════════════════════════╗${NC}" | ||
| echo -e "${BOLD}${GREEN}║ FULL DEVELOPMENT LOOP - COMPLETE ║${NC}" | ||
| echo -e "${BOLD}${GREEN}╚════════════════════════════════════════════════════════════╝${NC}" | ||
| echo "" | ||
|
|
||
| load_state 2>/dev/null || true | ||
|
|
||
| echo -e "${GREEN}All phases completed successfully!${NC}" | ||
| echo "" | ||
| echo "Summary:" | ||
| echo " - Task: Implemented" | ||
| echo " - Preflight: Passed" | ||
| echo " - PR: #${PR_NUMBER:-unknown} merged" | ||
| echo " - Postflight: Healthy" | ||
| if is_aidevops_repo; then | ||
| echo " - Deploy: Complete" | ||
| fi | ||
| echo "" | ||
|
|
||
| clear_state | ||
|
|
||
| echo "<promise>FULL_LOOP_COMPLETE</promise>" | ||
|
|
||
| return 0 | ||
| } |
There was a problem hiding this comment.
cmd_complete summary currently over-promises (Passed/Merged/Healthy) even when skipped/unknown.
Proposed fix (make summary conditional and verify PR state when possible)
cmd_complete() {
@@
echo "Summary:"
echo " - Task: Implemented"
- echo " - Preflight: Passed"
- echo " - PR: #${PR_NUMBER:-unknown} merged"
- echo " - Postflight: Healthy"
+ if [[ "${SKIP_PREFLIGHT:-false}" == "true" ]]; then
+ echo " - Preflight: Skipped"
+ else
+ echo " - Preflight: Completed"
+ fi
+ if command -v gh >/dev/null 2>&1 && [[ -n "${PR_NUMBER:-}" ]]; then
+ local pr_state="UNKNOWN"
+ pr_state=$(gh pr view "$PR_NUMBER" --json state --jq '.state' 2>/dev/null || echo "UNKNOWN")
+ echo " - PR: #$PR_NUMBER ($pr_state)"
+ else
+ echo " - PR: #${PR_NUMBER:-unknown}"
+ fi
+ if [[ "${SKIP_POSTFLIGHT:-false}" == "true" ]]; then
+ echo " - Postflight: Skipped"
+ else
+ echo " - Postflight: Completed"
+ fi
if is_aidevops_repo; then
echo " - Deploy: Complete"
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.
| cmd_complete() { | |
| echo "" | |
| echo -e "${BOLD}${GREEN}╔════════════════════════════════════════════════════════════╗${NC}" | |
| echo -e "${BOLD}${GREEN}║ FULL DEVELOPMENT LOOP - COMPLETE ║${NC}" | |
| echo -e "${BOLD}${GREEN}╚════════════════════════════════════════════════════════════╝${NC}" | |
| echo "" | |
| load_state 2>/dev/null || true | |
| echo -e "${GREEN}All phases completed successfully!${NC}" | |
| echo "" | |
| echo "Summary:" | |
| echo " - Task: Implemented" | |
| echo " - Preflight: Passed" | |
| echo " - PR: #${PR_NUMBER:-unknown} merged" | |
| echo " - Postflight: Healthy" | |
| if is_aidevops_repo; then | |
| echo " - Deploy: Complete" | |
| fi | |
| echo "" | |
| clear_state | |
| echo "<promise>FULL_LOOP_COMPLETE</promise>" | |
| return 0 | |
| } | |
| cmd_complete() { | |
| echo "" | |
| echo -e "${BOLD}${GREEN}╔════════════════════════════════════════════════════════════╗${NC}" | |
| echo -e "${BOLD}${GREEN}║ FULL DEVELOPMENT LOOP - COMPLETE ║${NC}" | |
| echo -e "${BOLD}${GREEN}╚════════════════════════════════════════════════════════════╝${NC}" | |
| echo "" | |
| load_state 2>/dev/null || true | |
| echo -e "${GREEN}All phases completed successfully!${NC}" | |
| echo "" | |
| echo "Summary:" | |
| echo " - Task: Implemented" | |
| if [[ "${SKIP_PREFLIGHT:-false}" == "true" ]]; then | |
| echo " - Preflight: Skipped" | |
| else | |
| echo " - Preflight: Completed" | |
| fi | |
| if command -v gh >/dev/null 2>&1 && [[ -n "${PR_NUMBER:-}" ]]; then | |
| local pr_state="UNKNOWN" | |
| pr_state=$(gh pr view "$PR_NUMBER" --json state --jq '.state' 2>/dev/null || echo "UNKNOWN") | |
| echo " - PR: #$PR_NUMBER ($pr_state)" | |
| else | |
| echo " - PR: #${PR_NUMBER:-unknown}" | |
| fi | |
| if [[ "${SKIP_POSTFLIGHT:-false}" == "true" ]]; then | |
| echo " - Postflight: Skipped" | |
| else | |
| echo " - Postflight: Completed" | |
| fi | |
| if is_aidevops_repo; then | |
| echo " - Deploy: Complete" | |
| fi | |
| echo "" | |
| clear_state | |
| echo "<promise>FULL_LOOP_COMPLETE</promise>" | |
| return 0 | |
| } |
🤖 Prompt for AI Agents
In @.agent/scripts/full-loop-helper.sh around lines 628 - 654, The cmd_complete
function currently prints definitive status lines
(Passed/Merged/Healthy/Complete) even when phases were skipped or PR_NUMBER is
unknown; modify cmd_complete to load_state (using load_state) and conditionally
print each summary line only if corresponding state flags exist or checks
succeed (e.g., only print "Preflight: Passed" if a saved preflight status is
present, only print "PR: #... merged" when PR_NUMBER is set and verify merge
state if possible, only print "Postflight: Healthy" if postflight status
indicates healthy, and only print "Deploy: Complete" when is_aidevops_repo
returns true and a deploy flag exists); ensure clear_state is still called at
the end and keep the final promise tag but reflect actual known states rather
than hardcoded successes.
| # Output as JSON | ||
| output_json() { | ||
| local project_root="$1" | ||
|
|
||
| local branch todo_status ralph_status adherence changes | ||
| branch=$(get_branch) | ||
| todo_status=$(get_todo_status "$project_root") | ||
| ralph_status=$(get_ralph_status "$project_root") | ||
| adherence=$(check_workflow_adherence "$project_root") | ||
| changes=$(get_uncommitted_changes) | ||
|
|
||
| # Extract values using sed for reliable parsing | ||
| local completed incomplete in_progress staged unstaged | ||
| completed=$(echo "$todo_status" | sed -n 's/.*completed:\([0-9]*\).*/\1/p') | ||
| incomplete=$(echo "$todo_status" | sed -n 's/.*incomplete:\([0-9]*\).*/\1/p') | ||
| in_progress=$(echo "$todo_status" | sed -n 's/.*in_progress:\([0-9]*\).*/\1/p') | ||
| staged=$(echo "$changes" | sed -n 's/^staged:\([0-9]*\),.*/\1/p') | ||
| unstaged=$(echo "$changes" | sed -n 's/.*,unstaged:\([0-9]*\)$/\1/p') | ||
|
|
||
| cat <<EOF | ||
| { | ||
| "project": "$project_root", | ||
| "branch": "$branch", | ||
| "timestamp": "$(date -u '+%Y-%m-%dT%H:%M:%SZ')", | ||
| "todo": { | ||
| "completed": ${completed:-0}, | ||
| "incomplete": ${incomplete:-0}, | ||
| "in_progress": ${in_progress:-0} | ||
| }, | ||
| "changes": { | ||
| "staged": ${staged:-0}, | ||
| "unstaged": ${unstaged:-0} | ||
| }, | ||
| "ralph_loop": { | ||
| "active": $(echo "$ralph_status" | grep -q 'active:true' && echo "true" || echo "false") | ||
| }, | ||
| "workflow": { | ||
| "on_protected_branch": $(is_protected_branch && echo "true" || echo "false") | ||
| } | ||
| } | ||
| EOF | ||
| return 0 | ||
| } |
There was a problem hiding this comment.
JSON output lacks proper escaping for special characters.
The JSON generation directly interpolates shell variables without escaping quotes, backslashes, or control characters. If project_root or branch contains special characters like ", \, or newlines, the JSON output will be malformed.
🔒 Proposed fix using jq
Replace the heredoc JSON with jq-based generation for proper escaping:
- cat <<EOF
-{
- "project": "$project_root",
- "branch": "$branch",
- "timestamp": "$(date -u '+%Y-%m-%dT%H:%M:%SZ')",
- "todo": {
- "completed": ${completed:-0},
- "incomplete": ${incomplete:-0},
- "in_progress": ${in_progress:-0}
- },
- "changes": {
- "staged": ${staged:-0},
- "unstaged": ${unstaged:-0}
- },
- "ralph_loop": {
- "active": $(echo "$ralph_status" | grep -q 'active:true' && echo "true" || echo "false")
- },
- "workflow": {
- "on_protected_branch": $(is_protected_branch && echo "true" || echo "false")
- }
-}
-EOF
+ local ralph_active="false"
+ echo "$ralph_status" | grep -q 'active:true' && ralph_active="true"
+
+ local protected="false"
+ is_protected_branch && protected="true"
+
+ jq -n \
+ --arg project "$project_root" \
+ --arg branch "$branch" \
+ --arg timestamp "$(date -u '+%Y-%m-%dT%H:%M:%SZ')" \
+ --argjson completed "${completed:-0}" \
+ --argjson incomplete "${incomplete:-0}" \
+ --argjson in_progress "${in_progress:-0}" \
+ --argjson staged "${staged:-0}" \
+ --argjson unstaged "${unstaged:-0}" \
+ --argjson ralph_active "$ralph_active" \
+ --argjson on_protected_branch "$protected" \
+ '{
+ project: $project,
+ branch: $branch,
+ timestamp: $timestamp,
+ todo: {
+ completed: $completed,
+ incomplete: $incomplete,
+ in_progress: $in_progress
+ },
+ changes: {
+ staged: $staged,
+ unstaged: $unstaged
+ },
+ ralph_loop: {
+ active: $ralph_active
+ },
+ workflow: {
+ on_protected_branch: $on_protected_branch
+ }
+ }'This ensures all string values are properly escaped and numeric values are correctly typed.
| | Practice | Check | Status | | ||
| |----------|-------|--------| | ||
| | Pre-edit git check | On feature branch, not main | Required | | ||
| | TODO tracking | Tasks logged in TODO.md | Recommended | | ||
| | Commit hygiene | Atomic commits, clear messages | Required | | ||
| | Quality checks | Linters run before commit | Recommended | | ||
| | Documentation | Changes documented where needed | Situational | | ||
|
|
||
| **Check commands**: | ||
|
|
||
| ```bash | ||
| # Verify not on main | ||
| [[ "$(git branch --show-current)" != "main" ]] && echo "OK: Feature branch" || echo "WARN: On main" | ||
|
|
||
| # Check commit message quality | ||
| git log --oneline -5 | while read line; do | ||
| [[ ${#line} -gt 10 ]] && echo "OK: $line" || echo "WARN: Short message" | ||
| done | ||
|
|
||
| # Check for TODO.md updates | ||
| git diff --name-only HEAD~5 | grep -q "TODO.md" && echo "OK: TODO.md updated" || echo "INFO: No TODO.md changes" | ||
| ``` |
There was a problem hiding this comment.
Workflow adherence example can fail on short history / shallow clones.
git diff --name-only HEAD~5 will error if the repo has <5 commits (or shallow history). Suggest guarding (e.g., fall back to HEAD~1 or compare against merge-base).
🤖 Prompt for AI Agents
In @.agent/workflows/session-review.md around lines 113 - 134, The check using
git diff --name-only HEAD~5 can fail in shallow or short-history repos; update
the workflow to detect commit count and fall back to a safe reference (e.g., use
HEAD~1 or git merge-base --fork-point/merge-base with origin/main) before
calling git diff; specifically modify the step that runs "git diff --name-only
HEAD~5" to first compute a valid base (check commit count or compute merge-base)
and then call git diff with that computed base so the TODO.md check never errors
on shallow histories.
|
🔍 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: Sun Jan 11 03:25:41 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
1 participant
Summary
This PR adds two new commands to enhance AI-assisted development workflows:
1.
/session-review- Session Completeness AuditingReviews the current session for:
Files:
.agent/workflows/session-review.md- Main workflow documentation.agent/scripts/session-review-helper.sh- Helper script with gather/summary/json commands.agent/scripts/commands/session-review.md- Slash command definition2.
/full-loop- End-to-End Development AutomationChains the complete development workflow:
Features:
setup.shdeploymentFiles:
.agent/scripts/full-loop-helper.sh- Orchestrator script.agent/scripts/commands/full-loop.md- Slash command definition.agent/workflows/ralph-loop.md- Updated with full loop documentationHuman Decision Points
Testing
session-review-helper.sh gather/summary/jsontestedfull-loop-helper.sh help/status/dry-runtestedSummary by CodeRabbit
Release Notes
/session-reviewcommand: Conduct comprehensive session audits with context gathering, objective completion assessment, workflow validation, knowledge capture, and actionable recommendations/full-loopcommand: Execute automated end-to-end development workflows with automatic phase progression, state tracking, and resumable execution across multiple workflow stages✏️ Tip: You can customize this high-level summary in your review settings.