feat(supervisor): faster PR merge pipeline with parallel CI + review triage (t219) #922
Conversation
…triage (t219) Optimizations: 1. Fast-path for clean PRs: Skip review_triage state when CI is green and zero review threads exist 2. Multi-stage transitions: Already supported - pr_review → merging → merged → deploying → deployed in single pulse 3. Timing metrics: Track time spent in each stage (pr_review, review_triage, merging, deploying) and log to proof-log.jsonl for pipeline latency analysis Impact: - Clean PRs (CI green + no review comments) now go from complete → deployed in 1-2 pulses instead of 3+ - Timing data enables data-driven optimization of bottlenecks - Fast-path marked in triage_result with fast_path:true flag Related: t218 (proof-log), t148 (review triage)
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
WalkthroughAdded lifecycle timing instrumentation to the PR pipeline supervisor script by introducing a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Tue Feb 10 05:46:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.agents/scripts/supervisor-helper.sh (1)
6526-6578:⚠️ Potential issue | 🟠 MajorGate fast-path merge on successful review-thread verification.
The fast-path currently treats
check_review_threadsfailures (API outage, auth issues) as zero threads, triggering immediate merge with potentially unresolved feedback. This creates an inconsistency with the regular triage path (line 6682), which does not suppress errors. Capture the return code and only merge if threads are successfully fetched and empty; otherwise fall back toreview_triage.Proposed fix
- threads_json_fastpath=$(check_review_threads "$repo_slug_fastpath" "$pr_number_fastpath" 2>/dev/null || echo "[]") - local thread_count_fastpath - thread_count_fastpath=$(echo "$threads_json_fastpath" | jq 'length' 2>/dev/null || echo "0") - - if [[ "$thread_count_fastpath" -eq 0 ]]; then + threads_json_fastpath=$(check_review_threads "$repo_slug_fastpath" "$pr_number_fastpath" 2>/dev/null) + local threads_rc=$? + local thread_count_fastpath=0 + if [[ "$threads_rc" -eq 0 ]]; then + thread_count_fastpath=$(echo "$threads_json_fastpath" | jq 'length' 2>/dev/null || echo "0") + fi + + if [[ "$threads_rc" -eq 0 && "$thread_count_fastpath" -eq 0 ]]; then log_info "Fast-path: CI green + zero review threads - skipping review_triage, going directly to merge" if [[ "$dry_run" == "false" ]]; then db "$SUPERVISOR_DB" "UPDATE tasks SET triage_result = '{\"action\":\"merge\",\"threads\":0,\"fast_path\":true}' WHERE id = '$escaped_id';" cmd_transition "$task_id" "merging" 2>>"$SUPERVISOR_LOG" || true fi tstatus="merging" else - # Has review threads - go through normal triage + # Has review threads OR verification failed — go through normal triage if [[ "$dry_run" == "false" ]]; then cmd_transition "$task_id" "review_triage" 2>>"$SUPERVISOR_LOG" || true fi tstatus="review_triage" fi
🤖 Fix all issues with AI agents
In @.agents/scripts/supervisor-helper.sh:
- Around line 4548-4554: Replace the basic-regex grep call that sets
task_tool_count with an extended-regex usage: update the grep invocation that
assigns task_tool_count (the line containing task_tool_count=$(grep -c
'mcp_task\|"tool_name":"task"\|"name":"task"' "$log_file" 2>/dev/null || echo
0)) to use grep -E and unescaped pipe characters (e.g. grep -c -E
'mcp_task|"tool_name":"task"|"name":"task"' ...) so it matches the codebase
convention and improves readability; apply the same change to the analogous
occurrence referenced (lines around 8986-8992).
| # Task tool parallelism tracking (t217): detect whether the worker used the | ||
| # Task tool (mcp_task) to spawn sub-agents for parallel work. This is a | ||
| # heuristic quality signal — workers that parallelise independent subtasks | ||
| # are more efficient. Logged for pattern tracking and supervisor dashboards. | ||
| local task_tool_count=0 | ||
| task_tool_count=$(grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"' "$log_file" 2>/dev/null || echo 0) | ||
| echo "task_tool_count=$task_tool_count" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and get its basic info
ls -la .agents/scripts/supervisor-helper.sh 2>&1 || echo "File not found"Repository: marcusquinn/aidevops
Length of output: 151
🏁 Script executed:
# Check the actual content at the specified line ranges
echo "=== Lines 4548-4554 ===" && sed -n '4548,4554p' .agents/scripts/supervisor-helper.sh
echo "=== Lines 8986-8992 ===" && sed -n '8986,8992p' .agents/scripts/supervisor-helper.shRepository: marcusquinn/aidevops
Length of output: 1093
🏁 Script executed:
# Test grep behavior with basic regex (\|) vs extended regex (|)
# Create test data with mcp_task, "tool_name":"task", and "name":"task" patterns
test_data='log entry with mcp_task here
another line with "tool_name":"task"
more log with "name":"task" in it'
echo "=== Testing with basic regex (escaped pipes \|) ==="
echo "$test_data" | grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"' || echo "Count: 0 or grep found no matches"
echo "=== Testing with extended regex (-E, unescaped pipes) ==="
echo "$test_data" | grep -cE 'mcp_task|"tool_name":"task"|"name":"task"' || echo "Count: 0 or grep found no matches"
echo "=== Verify both return actual counts ==="
echo "$test_data" | grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"'
echo "$test_data" | grep -cE 'mcp_task|"tool_name":"task"|"name":"task"'Repository: marcusquinn/aidevops
Length of output: 222
🏁 Script executed:
# Count total instances of this grep pattern in the file to ensure we have all of them
rg -n 'grep.*mcp_task' .agents/scripts/supervisor-helper.sh | head -20Repository: marcusquinn/aidevops
Length of output: 299
🏁 Script executed:
# Verify the critical point: test literal | in basic regex (should NOT work)
test_data='log entry with mcp_task here
another line with "tool_name":"task"
more log with "name":"task" in it'
echo "=== Testing literal | in basic regex (should NOT match) ==="
echo "$test_data" | grep -c 'mcp_task|"tool_name":"task"|"name":"task"' 2>&1
echo "=== Count using literal pipes ==="
echo "$test_data" | grep 'mcp_task|"tool_name":"task"|"name":"task"'
echo "=== Confirm escaped pipes work in basic regex ==="
echo "$test_data" | grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"'Repository: marcusquinn/aidevops
Length of output: 211
🏁 Script executed:
# Test ShellCheck on the grep patterns to see if there's a style warning
cat > /tmp/test_grep.sh << 'EOF'
#!/bin/bash
log_file="test.log"
# Original with escaped pipes (basic regex)
task_tool_count=$(grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"' "$log_file" 2>/dev/null || echo 0)
# Suggested with -E and unescaped pipes (extended regex)
task_tool_count=$(grep -cE 'mcp_task|"tool_name":"task"|"name":"task"' "$log_file" 2>/dev/null || echo 0)
EOF
# Check if ShellCheck is available and run it
if command -v shellcheck &> /dev/null; then
shellcheck /tmp/test_grep.sh
else
echo "ShellCheck not available, but both patterns are syntactically valid."
fiRepository: marcusquinn/aidevops
Length of output: 412
🏁 Script executed:
# Verify which grep variants are idiomatic/recommended in the codebase
echo "=== Check grep usage patterns in supervisor-helper.sh ==="
rg 'grep\s+-' .agents/scripts/supervisor-helper.sh | head -15Repository: marcusquinn/aidevops
Length of output: 1767
Align grep syntax with codebase conventions.
The current code uses basic regex with escaped pipes (\|), which works correctly. However, the codebase consistently uses grep -E for extended regex patterns (see grep usage throughout the file). Switch to -E with unescaped pipes for readability and consistency.
🔧 Suggested fix
- task_tool_count=$(grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"' "$log_file" 2>/dev/null || echo 0)
+ task_tool_count=$(grep -cE 'mcp_task|"tool_name":"task"|"name":"task"' "$log_file" 2>/dev/null || echo 0)- task_tool_count=$(grep -cE 'mcp_task\|"tool_name":"task"\|"name":"task"' "$log_file" 2>/dev/null || echo 0)
+ task_tool_count=$(grep -cE 'mcp_task|"tool_name":"task"|"name":"task"' "$log_file" 2>/dev/null || echo 0)Also applies to: 8986-8992
🤖 Prompt for AI Agents
In @.agents/scripts/supervisor-helper.sh around lines 4548 - 4554, Replace the
basic-regex grep call that sets task_tool_count with an extended-regex usage:
update the grep invocation that assigns task_tool_count (the line containing
task_tool_count=$(grep -c 'mcp_task\|"tool_name":"task"\|"name":"task"'
"$log_file" 2>/dev/null || echo 0)) to use grep -E and unescaped pipe characters
(e.g. grep -c -E 'mcp_task|"tool_name":"task"|"name":"task"' ...) so it matches
the codebase convention and improves readability; apply the same change to the
analogous occurrence referenced (lines around 8986-8992).
1 participant
Summary
Optimizes the PR lifecycle in the supervisor to reduce time from PR creation to deployment by implementing:
review_triagestate entirely and go directly frompr_review→mergingpr_review→merging→merged→deploying→deployed) in a single pulseproof-log.jsonlfor pipeline latency analysisImpact
complete→deployedin 1-2 pulses instead of 3+triage_resultwithfast_path:trueflag for analyticsImplementation Details
Fast-Path Logic
When a PR reaches
pr_reviewstate withready_to_mergestatus:skip_review_triageis enabled (bypass)merging(skipreview_triage)Timing Metrics
Each stage tracks:
pr_review: Time from entering pr_review to exiting (includes CI wait)review_triage: Time spent triaging review feedbackmerging: Time to merge PRdeploying: Time to run postflight + deploy + cleanuptotal: Total lifecycle timeLogged to
proof-log.jsonlas:{ "timestamp": "2026-02-10T05:30:00Z", "task_id": "t219", "event": "pr_lifecycle_timing", "stages": { "pr_review": "5s", "merging": "3s", "deploying": "12s", "total": "20s" } }Testing
bash -ncmd_pr_lifecycleflowcheck_pr_statusvalidation)Related
Closes #914
Summary by CodeRabbit