From 58609548962d3fe8058ab4837c8d1201895a673b Mon Sep 17 00:00:00 2001 From: marcusquinn <6428977+marcusquinn@users.noreply.github.com> Date: Fri, 20 Feb 2026 04:13:05 +0000 Subject: [PATCH 1/2] fix: crash-resilient evaluation with SIGTERM rollback (t1269) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Phase 1 evaluation runs inline in the pulse loop. When the pulse process is killed mid-evaluation (SIGTERM from cron, OOM, system sleep), the task is stranded in 'evaluating' with no live process. Phase 0.7 catches these but only after 120-600s grace periods, wasting concurrency slots. Analysis of 205 stale recovery events showed: - eval_process_died: 36 events (avg 639s stuck) - worker_failed_before_eval: 34 events (avg 614s stuck) - pulse_killed_after_pr_persist: 33 events (avg 781s stuck) - worker_oom_killed: 24 events (avg 1092s stuck) Fix: Install a SIGTERM/SIGINT trap inside the Phase 1 evaluation loop that tracks which task is currently being evaluated. On pulse death, the trap rolls back the in-progress task from 'evaluating' to its pre-eval state (running/dispatched), so the next pulse can re-evaluate immediately without waiting for the grace period. The rollback SQL uses AND status='evaluating' as a safety guard — if the task was already transitioned to its final state before the signal, the rollback is a no-op. --- .agents/scripts/supervisor/pulse.sh | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/.agents/scripts/supervisor/pulse.sh b/.agents/scripts/supervisor/pulse.sh index 6226c8588a..9cc7451183 100755 --- a/.agents/scripts/supervisor/pulse.sh +++ b/.agents/scripts/supervisor/pulse.sh @@ -1314,7 +1314,29 @@ cmd_pulse() { local failed_count=0 local dispatched_count=0 + # t1269: Track which task is currently being evaluated so we can roll back + # on pulse death (SIGTERM/SIGINT). Without this, a killed pulse leaves the + # task stranded in 'evaluating' until Phase 0.7 detects it (120-600s). + # With this trap, the task is rolled back to its pre-evaluation state + # immediately, so the next pulse can re-evaluate it without delay. + local _phase1_evaluating_tid="" + local _phase1_pre_eval_state="" + # shellcheck disable=SC2329 # invoked indirectly via trap + _phase1_cleanup_on_signal() { + if [[ -n "$_phase1_evaluating_tid" && -n "$_phase1_pre_eval_state" ]]; then + log_warn " Phase 1 (t1269): pulse killed during evaluation of $_phase1_evaluating_tid — rolling back to $_phase1_pre_eval_state" + db "$SUPERVISOR_DB" "UPDATE tasks SET status = '$(sql_escape "$_phase1_pre_eval_state")', updated_at = strftime('%Y-%m-%dT%H:%M:%SZ','now'), error = 'pulse_killed_mid_eval_rollback_t1269' WHERE id = '$(sql_escape "$_phase1_evaluating_tid")' AND status = 'evaluating';" 2>/dev/null || true + _phase1_evaluating_tid="" + _phase1_pre_eval_state="" + fi + } + if [[ -n "$running_tasks" ]]; then + # Install signal trap for crash-resilient evaluation (t1269). + # Chain with existing EXIT trap (release_pulse_lock) by saving and restoring. + # shellcheck disable=SC2064 # intentional: expand SUPERVISOR_DIR at definition time + trap "_phase1_cleanup_on_signal; release_pulse_lock; rm -f '${SUPERVISOR_DIR}/MODELS.md.tmp' 2>/dev/null || true" TERM INT + while IFS='|' read -r tid _; do # Check if worker process is still alive local pid_file="$SUPERVISOR_DIR/pids/${tid}.pid" @@ -1340,11 +1362,19 @@ cmd_pulse() { if [[ "$current_task_state" == "evaluating" ]]; then log_info " $tid: stuck in evaluating (AI eval likely timed out), re-evaluating without AI..." + # t1269: Record pre-eval state for rollback — already evaluating, so + # rolling back to 'running' is the safest option (worker is dead, but + # 'running' lets Phase 1 re-evaluate on next pulse without grace delay). + _phase1_pre_eval_state="running" else log_info " $tid: worker finished, evaluating..." + # t1269: Record pre-eval state before transitioning + _phase1_pre_eval_state="$current_task_state" # Transition to evaluating cmd_transition "$tid" "evaluating" 2>>"$SUPERVISOR_LOG" || true fi + # t1269: Mark this task as actively being evaluated + _phase1_evaluating_tid="$tid" # Get task description for memory context (t128.6) local tid_desc @@ -1437,6 +1467,8 @@ cmd_pulse() { cleanup_worker_processes "$tid" # Success pattern already stored by scan_orphaned_pr_for_task handle_diagnostic_completion "$tid" 2>>"$SUPERVISOR_LOG" || true + _phase1_evaluating_tid="" # t1269: clear before continue + _phase1_pre_eval_state="" continue fi fi @@ -1463,6 +1495,8 @@ cmd_pulse() { # Add escalated:model label (original model that failed quality gate) (t1010) add_model_label "$tid" "escalated" "$tid_model" "${tid_repo:-.}" 2>>"$SUPERVISOR_LOG" || true send_task_notification "$tid" "escalated" "Re-queued with $escalated_model" 2>>"$SUPERVISOR_LOG" || true + _phase1_evaluating_tid="" # t1269: clear before continue + _phase1_pre_eval_state="" continue fi @@ -1549,6 +1583,8 @@ cmd_pulse() { # retry attempts. Leave in retrying state for deferred retry loop. if [[ "$outcome_detail" == "backend_quota_error" || "$outcome_detail" == "backend_infrastructure_error" ]]; then log_warn " $tid: backend issue ($outcome_detail), deferring re-prompt to next pulse" + _phase1_evaluating_tid="" # t1269: clear before continue + _phase1_pre_eval_state="" continue fi # Prompt-repeat retry strategy (t1097): before escalating to a more @@ -1563,6 +1599,8 @@ cmd_pulse() { if [[ "$pr_rc" -eq 0 ]]; then dispatched_count=$((dispatched_count + 1)) log_info " $tid: prompt-repeat dispatched successfully" + _phase1_evaluating_tid="" # t1269: clear before continue + _phase1_pre_eval_state="" continue fi log_warn " $tid: prompt-repeat dispatch failed (rc=$pr_rc), falling through to model escalation" @@ -1685,7 +1723,19 @@ cmd_pulse() { fi ;; esac + + # t1269: Clear evaluation tracking — task has been fully processed. + # If the pulse is killed after this point, the task is already in its + # final state (complete/retrying/blocked/failed/queued) and doesn't + # need rollback. + _phase1_evaluating_tid="" + _phase1_pre_eval_state="" done <<<"$running_tasks" + + # t1269: Restore original signal trap after evaluation loop completes. + # The EXIT trap (release_pulse_lock) is still active from the outer scope. + # shellcheck disable=SC2064 # intentional: expand SUPERVISOR_DIR at definition time + trap "release_pulse_lock; rm -f '${SUPERVISOR_DIR}/MODELS.md.tmp' 2>/dev/null || true" TERM INT fi # Phase 1b: Re-prompt stale retrying tasks (t153-pre-diag-1) From 660d38f26425d09923349e790935bd978a84807c Mon Sep 17 00:00:00 2001 From: marcusquinn <6428977+marcusquinn@users.noreply.github.com> Date: Fri, 20 Feb 2026 04:39:40 +0000 Subject: [PATCH 2/2] fix: address review feedback for t1269 Close SIGTERM race window by setting _phase1_evaluating_tid before cmd_transition in both code paths (already-evaluating and new-evaluation). The SQL guard (AND status = 'evaluating') in the cleanup handler makes a premature rollback a no-op if the DB transition hasn't committed yet. --- .agents/scripts/supervisor/pulse.sh | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.agents/scripts/supervisor/pulse.sh b/.agents/scripts/supervisor/pulse.sh index 9cc7451183..a2b44c469b 100755 --- a/.agents/scripts/supervisor/pulse.sh +++ b/.agents/scripts/supervisor/pulse.sh @@ -1366,15 +1366,23 @@ cmd_pulse() { # rolling back to 'running' is the safest option (worker is dead, but # 'running' lets Phase 1 re-evaluate on next pulse without grace delay). _phase1_pre_eval_state="running" + # t1269: Mark this task as actively being evaluated BEFORE any state + # transition, so the SIGTERM cleanup handler always knows which task + # to roll back. The SQL guard (AND status = 'evaluating') makes a + # premature rollback a no-op if the transition hasn't committed yet. + _phase1_evaluating_tid="$tid" else log_info " $tid: worker finished, evaluating..." # t1269: Record pre-eval state before transitioning _phase1_pre_eval_state="$current_task_state" + # t1269: Mark this task as actively being evaluated BEFORE the + # transition to close the SIGTERM race window. If SIGTERM arrives + # after this but before cmd_transition, the cleanup handler's SQL + # guard (AND status = 'evaluating') makes the rollback a no-op. + _phase1_evaluating_tid="$tid" # Transition to evaluating cmd_transition "$tid" "evaluating" 2>>"$SUPERVISOR_LOG" || true fi - # t1269: Mark this task as actively being evaluated - _phase1_evaluating_tid="$tid" # Get task description for memory context (t128.6) local tid_desc