Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 34 additions & 21 deletions .agents/scripts/supervisor/todo-sync.sh
Original file line number Diff line number Diff line change
Expand Up @@ -269,40 +269,53 @@ process_verify_queue() {

local verified_count=0
local failed_count=0
local auto_verified_count=0
local max_auto_verify_per_pulse=50
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

According to the repository style guide (line 39), constants should be named using UPPER_SNAKE_CASE. Please rename max_auto_verify_per_pulse to MAX_AUTO_VERIFY_PER_PULSE and update its usage on line 306 accordingly.

Suggested change
local max_auto_verify_per_pulse=50
local MAX_AUTO_VERIFY_PER_PULSE=50
References
  1. Variables: UPPER_SNAKE for constants/env vars, lower_snake for locals (link)


while IFS='|' read -r tid trepo _tpr; do
[[ -z "$tid" ]] && continue

local verify_file="$trepo/todo/VERIFY.md"
if [[ ! -f "$verify_file" ]]; then
continue
fi
local has_entry=false

# Check if there's a pending verify entry for this task
if ! grep -q "^- \[ \] v[0-9]* $tid " "$verify_file" 2>/dev/null; then
continue
if [[ -f "$verify_file" ]] && grep -q "^- \[ \] v[0-9]* $tid " "$verify_file" 2>/dev/null; then
has_entry=true
fi

log_info " $tid: running verification checks"
cmd_transition "$tid" "verifying" 2>>"$SUPERVISOR_LOG" || {
log_warn " $tid: failed to transition to verifying"
continue
}
if [[ "$has_entry" == "true" ]]; then
Comment on lines +279 to +285
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The use of the has_entry flag can be simplified. Instead of setting a flag in one if block and then immediately checking it in another, you can use the condition directly in the main if/else structure. This makes the control flow more direct and removes the need for a temporary variable.

Suggested change
local has_entry=false
# Check if there's a pending verify entry for this task
if ! grep -q "^- \[ \] v[0-9]* $tid " "$verify_file" 2>/dev/null; then
continue
if [[ -f "$verify_file" ]] && grep -q "^- \[ \] v[0-9]* $tid " "$verify_file" 2>/dev/null; then
has_entry=true
fi
log_info " $tid: running verification checks"
cmd_transition "$tid" "verifying" 2>>"$SUPERVISOR_LOG" || {
log_warn " $tid: failed to transition to verifying"
continue
}
if [[ "$has_entry" == "true" ]]; then
if [[ -f "$verify_file" ]] && grep -q "^- \[ \] v[0-9]* $tid " "$verify_file" 2>/dev/null; then

# Has VERIFY.md entry — run the defined checks
log_info " $tid: running verification checks"
cmd_transition "$tid" "verifying" 2>>"$SUPERVISOR_LOG" || {
log_warn " $tid: failed to transition to verifying"
continue
}

if run_verify_checks "$tid" "$trepo"; then
cmd_transition "$tid" "verified" 2>>"$SUPERVISOR_LOG" || true
verified_count=$((verified_count + 1))
log_success " $tid: VERIFIED"
if run_verify_checks "$tid" "$trepo"; then
cmd_transition "$tid" "verified" 2>>"$SUPERVISOR_LOG" || true
verified_count=$((verified_count + 1))
log_success " $tid: VERIFIED"
else
cmd_transition "$tid" "verify_failed" 2>>"$SUPERVISOR_LOG" || true
failed_count=$((failed_count + 1))
log_warn " $tid: VERIFY FAILED"
send_task_notification "$tid" "verify_failed" "Post-merge verification failed" 2>>"$SUPERVISOR_LOG" || true
fi
else
cmd_transition "$tid" "verify_failed" 2>>"$SUPERVISOR_LOG" || true
failed_count=$((failed_count + 1))
log_warn " $tid: VERIFY FAILED"
send_task_notification "$tid" "verify_failed" "Post-merge verification failed" 2>>"$SUPERVISOR_LOG" || true
# No VERIFY.md entry — auto-verify (PR merged + CI passed is sufficient)
# Rate-limit to avoid overwhelming the state machine in one pulse
if [[ "$auto_verified_count" -ge "$max_auto_verify_per_pulse" ]]; then
continue
fi
cmd_transition "$tid" "verified" 2>>"$SUPERVISOR_LOG" || {
log_warn " $tid: failed to auto-verify"
continue
}
auto_verified_count=$((auto_verified_count + 1))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

For better observability, it would be helpful to add a log message when a task is auto-verified. This would be consistent with the manual verification path, which logs a success message, and would make it clearer in the logs which specific tasks were auto-verified.

Suggested change
auto_verified_count=$((auto_verified_count + 1))
auto_verified_count=$((auto_verified_count + 1))
log_info " $tid: auto-verified (no VERIFY.md entry)"

fi
done <<<"$deployed_tasks"

if [[ $((verified_count + failed_count)) -gt 0 ]]; then
log_info "Verification: $verified_count passed, $failed_count failed"
if [[ $((verified_count + failed_count + auto_verified_count)) -gt 0 ]]; then
log_info "Verification: $verified_count passed, $failed_count failed, $auto_verified_count auto-verified (no VERIFY.md entry)"
fi

return 0
Expand Down
Loading