Skip to main content

orch/engine/runner/
response_handler.rs

1//! Success-path response handler — commit, push, PR, token storage.
2//!
3//! Extracted from `runner/mod.rs`. Handles the `Ok(parsed)` arm of the parse
4//! result, including git operations and delegation storage. Also owns
5//! `write_result_json`.
6
7use crate::config;
8use crate::parser::AgentResponse;
9use crate::store;
10use crate::store::TaskStore;
11use std::io;
12use std::io::ErrorKind;
13use std::path::Path;
14use std::sync::Arc;
15
16use tokio::fs;
17
18use super::{agents, git_ops, response, worktree};
19use crate::engine::cooldown;
20use crate::engine::review;
21use crate::github::http::GhHttp;
22use crate::status_heuristics;
23use crate::store::Task;
24
25/// Write a structured `result.json` to the attempt directory for debugging.
26#[expect(
27    clippy::too_many_arguments,
28    reason = "each field of result.json is a separate argument"
29)]
30pub async fn write_result_json(
31    attempt_dir: &Path,
32    task_id: &str,
33    agent_name: &str,
34    model_name: Option<&str>,
35    exit_code: i32,
36    new_attempts: u32,
37    parse_result: &Result<agents::ParsedResponse, agents::AgentError>,
38    raw_stdout: &str,
39    raw_stderr: &str,
40) {
41    let result_json = match parse_result {
42        Ok(parsed) => {
43            serde_json::json!({
44                "outcome": "success",
45                "agent": agent_name,
46                "model": model_name.unwrap_or("default"),
47                "exit_code": exit_code,
48                "attempt": new_attempts,
49                "status": parsed.response.status,
50                "summary": parsed.response.summary,
51                "input_tokens": parsed.input_tokens,
52                "output_tokens": parsed.output_tokens,
53                "duration_ms": parsed.duration_ms,
54                "files": parsed.response.files,
55                "accomplished": parsed.response.accomplished,
56                "remaining": parsed.response.remaining,
57                "error": parsed.response.error,
58                "learnings": parsed.response.learnings,
59                "delegations": parsed.response.delegations.iter()
60                    .map(|d| serde_json::json!({"title": d.title, "body": d.body}))
61                    .collect::<Vec<_>>(),
62            })
63        }
64        Err(agent_err) => {
65            serde_json::json!({
66                "outcome": "error",
67                "agent": agent_name,
68                "model": model_name.unwrap_or("default"),
69                "exit_code": exit_code,
70                "attempt": new_attempts,
71                "error_class": agents::error_class_name(agent_err),
72                "error_message": agent_err.to_string(),
73                "stderr_tail": agents::patterns::safe_tail(raw_stderr, 2000),
74                "stdout_tail": agents::patterns::safe_tail(raw_stdout, 2000),
75            })
76        }
77    };
78
79    if let Err(e) = fs::write(
80        attempt_dir.join("result.json"),
81        serde_json::to_string_pretty(&result_json).unwrap_or_default(),
82    )
83    .await
84    {
85        tracing::debug!(task_id, ?e, "failed to write result.json");
86    }
87}
88
89/// Input to the pure status-decision function [`classify_final_status`].
90///
91/// The caller is responsible for pre-computing counter values (via store
92/// increments) and populating them here.  No I/O is performed inside the
93/// decision function itself, making it trivially unit-testable.
94#[derive(Default)]
95#[expect(
96    clippy::struct_excessive_bools,
97    reason = "each flag is an independent input to the status decision"
98)]
99struct DecisionInput<'a> {
100    /// Agent-reported status (e.g. `"done"`, `"in_progress"`).
101    agent_status: &'a str,
102    /// Push was attempted and commits existed, but the push error contained a
103    /// `workflow` scope complaint — non-retryable.
104    is_workflow_scope_failure: bool,
105    /// Push failure recovery attempted a rebase but failed (for example due to
106    /// conflicts). This is non-retryable without human intervention.
107    is_rebase_conflict_failure: bool,
108    /// Push was attempted (commits existed) but failed.
109    push_failed: bool,
110    /// Persistent push-failure counter *after* this run's increment.
111    /// Only meaningful when `push_failed` is `true`.
112    push_failures: u64,
113    /// A PR already exists (or was just created) for this task.
114    has_pr: bool,
115    /// The agent created sub-task delegations.
116    has_delegations: bool,
117    /// Commits were pushed to the remote branch successfully.
118    has_pushed: bool,
119    /// PR creation failed because the base branch was invalid (even after
120    /// GitHub API fallback). This is a terminal error — the task should be
121    /// blocked rather than re-dispatched.
122    is_pr_base_invalid: bool,
123    /// The task is external and requires a PR to be marked done.
124    /// Always false for internal tasks.
125    requires_pr: bool,
126    /// Persistent no-code-reroute counter *after* this run's increment.
127    no_code_reroutes: u64,
128    /// Maximum no-code reroutes before blocking (from config).
129    max_reroutes: u32,
130    /// True if the agent that just ran is the same as the one that produced
131    /// the previous no-code result. Same-agent loops should be blocked immediately.
132    is_same_agent: bool,
133    /// True when agent_status is "completed". Used to distinguish internal
134    /// completed tasks (mark done) from "done" tasks where the git ops pipeline
135    /// would have detected no-code reroutes. Pre-computed from agent_status
136    /// by the caller so this function stays pure.
137    is_completed_status: bool,
138    /// Agent explicitly returned blocked, but reason appears transient and retryable.
139    is_retryable_blocked: bool,
140    /// The task was re-dispatched to resolve a merge conflict and came back with
141    /// nothing ahead of the default branch, meaning the fix already landed and
142    /// the open PR is superseded. Mark done instead of counting a no-code run.
143    is_superseded_conflict: bool,
144}
145
146/// Determine the final task status from pre-computed state.
147///
148/// This is a pure function — it performs no I/O.  All store increments must be
149/// done by the caller **before** calling this function (so counters reflect the
150/// current run), and all store side-effects (clearing agent/model, writing
151/// last_error) must be applied **after** based on the returned status.
152fn classify_final_status(input: &DecisionInput<'_>) -> String {
153    if input.is_workflow_scope_failure || input.is_rebase_conflict_failure {
154        "blocked".to_string()
155    } else if input.push_failed {
156        if input.push_failures >= 3 {
157            "blocked".to_string()
158        } else {
159            "new".to_string()
160        }
161    } else if input.agent_status == "done" && input.has_pr {
162        "needs_review".to_string()
163    } else if input.agent_status == "done" && !input.has_pr && input.has_delegations {
164        "blocked".to_string()
165    } else if input.agent_status == "done"
166        && !input.has_pr
167        && input.has_pushed
168        && input.is_pr_base_invalid
169    {
170        // PR creation failed with an invalid base branch (even after GitHub API
171        // fallback).  This is a terminal configuration error — re-dispatching
172        // would just burn tokens on the same failure.
173        "blocked".to_string()
174    } else if input.agent_status == "done" && !input.has_pr && input.has_pushed {
175        "routed".to_string()
176    } else if input.is_superseded_conflict {
177        "done".to_string()
178    } else if input.agent_status == "done" && input.requires_pr {
179        // Same-agent loop detection: if this agent is the same as the one that
180        // produced the previous no-code result, block immediately (#2410, #2686).
181        // Otherwise, block if max reroutes exhausted, else reroute.
182        if input.is_same_agent || input.no_code_reroutes >= u64::from(input.max_reroutes) {
183            "blocked".to_string()
184        } else {
185            "new".to_string()
186        }
187    } else if input.agent_status == "done" && !input.has_pushed {
188        // Only internal tasks reach here — external done+!has_pr+!has_pushed is
189        // always caught by branch 6 (requires_pr = true for external tasks with
190        // !has_pr and done status), so is_external can never be true here.
191        // Internal tasks may legitimately produce no git-visible changes.
192        "done".to_string()
193    } else if input.is_completed_status && input.has_pr {
194        // Agent said "completed" and a PR was created — send to review.
195        "needs_review".to_string()
196    } else if input.is_completed_status && !input.has_pr && input.has_delegations {
197        // Agent said "completed" with delegations but no PR — blocked on children.
198        "blocked".to_string()
199    } else if input.is_completed_status
200        && !input.has_pr
201        && input.has_pushed
202        && input.is_pr_base_invalid
203    {
204        "blocked".to_string()
205    } else if input.is_completed_status && !input.has_pr && input.has_pushed {
206        // Agent said "completed", commits pushed but no PR — re-dispatch for PR.
207        "routed".to_string()
208    } else if input.is_completed_status && input.requires_pr {
209        // Agent said "completed" on external task requiring PR without a PR or pushes.
210        // Same-agent loop detection applies, then reroute or block.
211        if input.is_same_agent || input.no_code_reroutes >= u64::from(input.max_reroutes) {
212            "blocked".to_string()
213        } else {
214            "new".to_string()
215        }
216    } else if input.is_completed_status {
217        // Agent said "completed" on internal task — mark done.
218        // Git ops already ran and detected no commits, so this is a legitimate
219        // completion without code changes.
220        "done".to_string()
221    } else if input.agent_status == "blocked" && input.is_retryable_blocked {
222        "new".to_string()
223    } else if status_heuristics::status_looks_like_descriptive_completion(input.agent_status) {
224        "done".to_string()
225    } else {
226        input.agent_status.to_string()
227    }
228}
229
230// ── Internal context ──────────────────────────────────────────────────────────
231
232/// Bundles store references so helper functions don't repeat four parameters.
233struct StoreCtx<'a> {
234    store: &'a Option<Arc<TaskStore>>,
235    store_id_opt: Option<i64>,
236    repo: &'a str,
237    task_id: &'a str,
238}
239
240impl StoreCtx<'_> {
241    /// Write fields, using the pre-resolved `store_id` when available.
242    async fn set(&self, fields: &[(&str, serde_json::Value)]) {
243        if let Some(store_id) = self.store_id_opt {
244            let _ = store::store_set_by_id(self.store.as_ref(), store_id, fields).await;
245        } else {
246            store::store_set(self.store, self.repo, self.task_id, fields).await;
247        }
248    }
249
250    /// Load the full task record.
251    async fn get_task(&self) -> Option<Task> {
252        if let Some(store_id) = self.store_id_opt {
253            store::opt_store_get_task_by_id(self.store, store_id).await
254        } else {
255            store::opt_store_get_task(self.store, self.repo, self.task_id).await
256        }
257    }
258
259    /// Atomically increment a counter field.
260    async fn increment(&self, field: &str) -> anyhow::Result<u64> {
261        if let Some(store_id) = self.store_id_opt {
262            store::store_increment_by_id(self.store, store_id, field).await
263        } else {
264            store::store_increment(self.store, self.repo, self.task_id, field).await
265        }
266    }
267
268    /// Append an activity event to the task timeline.
269    async fn append_activity(
270        &self,
271        event_type: &str,
272        agent: Option<&str>,
273        model: Option<&str>,
274        details: Option<&serde_json::Value>,
275    ) {
276        if let Some(store_id) = self.store_id_opt {
277            if let Some(s) = self.store
278                && let Err(e) = s
279                    .append_activity(store_id, event_type, None, None, agent, model, details)
280                    .await
281            {
282                tracing::warn!(
283                    task_id = self.task_id,
284                    event_type,
285                    error = %e,
286                    "store append_activity failed"
287                );
288            }
289        } else {
290            store::store_log_activity(
291                self.store,
292                self.repo,
293                self.task_id,
294                event_type,
295                None,
296                None,
297                agent,
298                model,
299                details,
300            )
301            .await;
302        }
303    }
304}
305
306// ── Git operation result types ────────────────────────────────────────────────
307
308/// Outcome of the auto-commit → push → PR pipeline.
309#[derive(Default)]
310#[expect(
311    clippy::struct_excessive_bools,
312    reason = "each flag is an independent outcome of the git pipeline"
313)]
314struct GitOpsResult {
315    has_pr: bool,
316    has_pushed: bool,
317    has_commits: bool,
318    /// PR creation failed with an invalid base branch (after all retries
319    /// and the GitHub API fallback).  Used to block the task instead of
320    /// re-dispatching it indefinitely.
321    pr_base_invalid: bool,
322}
323
324/// Push-failure detection result.
325struct PushFailureState {
326    push_failed: bool,
327    is_workflow_scope_failure: bool,
328    is_rebase_conflict_failure: bool,
329    /// Post-increment failure counter (0 when no push was attempted or failed).
330    push_failures: u64,
331    /// Last error stored in the task record (used for workflow-scope detection).
332    stored_last_error: String,
333}
334
335// ── Private git-ops helpers ───────────────────────────────────────────────────
336
337/// Attempt auto-commit; record any error in the store.
338async fn auto_commit_changes(
339    ctx: &StoreCtx<'_>,
340    work_dir: &Path,
341    task_title: &str,
342    agent_name: &str,
343    new_attempts: u32,
344) {
345    if let Err(e) = git_ops::auto_commit(
346        work_dir,
347        ctx.task_id,
348        task_title,
349        agent_name,
350        new_attempts,
351        super::commit::When::Always,
352    )
353    .await
354    {
355        tracing::error!(task_id = ctx.task_id, error = ?e, "auto commit failed");
356        ctx.set(&[(
357            "last_error",
358            serde_json::json!(format!("auto commit failed: {e}")),
359        )])
360        .await;
361    }
362}
363
364/// Check whether commits exist ahead of the default branch.
365///
366/// When there are no commits, clears any stale push-failure error so that the
367/// review gate does not block an otherwise-approved task.
368async fn check_commits_and_clear_stale_errors(
369    ctx: &StoreCtx<'_>,
370    work_dir: &Path,
371    default_branch: &str,
372) -> bool {
373    let has_commits = git_ops::has_commits_ahead(work_dir, default_branch).await;
374    if !has_commits {
375        tracing::info!(
376            task_id = ctx.task_id,
377            "no commits ahead of default branch, skipping push + PR"
378        );
379        // Clear stale push failure from previous runs.
380        let last_err = ctx
381            .get_task()
382            .await
383            .map(|t| t.last_error)
384            .unwrap_or_default();
385        if last_err.contains("push failed") {
386            ctx.set(&[("last_error", serde_json::json!(""))]).await;
387        }
388    }
389    has_commits
390}
391
392/// Push the branch to the remote and log the activity.
393///
394/// On push failure due to diverged history (non-fast-forward), fetches the remote
395/// branch and rebases local commits on top, then pushes again. If rebase fails
396/// (conflicts), the task is blocked for human intervention.
397///
398/// Returns `PushResult::Success` on success, `PushResult::Failed` if push failed
399/// and recovery was not possible, or `PushResult::NoCommits` if the branch has no
400/// commits ahead of the default branch
401enum PushResult {
402    Success,
403    Failed,
404    NoCommits,       // agent made no code changes
405    WorktreeMissing, // retry after rebuilding missing worktree
406}
407
408fn is_missing_path_error(err: &anyhow::Error) -> bool {
409    err.chain().any(|cause| {
410        cause
411            .downcast_ref::<io::Error>()
412            .is_some_and(|ioe| ioe.kind() == ErrorKind::NotFound)
413    }) || err
414        .to_string()
415        .to_ascii_lowercase()
416        .contains("no such file or directory")
417}
418
419async fn recover_missing_worktree_for_push(
420    ctx: &StoreCtx<'_>,
421    wt: &worktree::WorktreeSetup,
422    task_title: &str,
423    agent_name: &str,
424    model_name: Option<&str>,
425) -> anyhow::Result<worktree::WorktreeSetup> {
426    tracing::warn!(
427        task_id = ctx.task_id,
428        worktree = %wt.work_dir.display(),
429        branch = wt.branch,
430        "worktree missing during push; attempting one-time recovery"
431    );
432    ctx.append_activity(
433        "push_recover_worktree",
434        Some(agent_name),
435        model_name,
436        Some(&serde_json::json!({
437            "status": "start",
438            "worktree": wt.work_dir.display().to_string(),
439            "branch": wt.branch,
440        })),
441    )
442    .await;
443
444    let recovered = worktree::setup_worktree(
445        ctx.task_id,
446        task_title,
447        &wt.main_project_dir,
448        ctx.store,
449        ctx.repo,
450    )
451    .await?;
452
453    if !recovered.work_dir.exists() {
454        anyhow::bail!(
455            "recovered worktree path is still missing: {}",
456            recovered.work_dir.display()
457        );
458    }
459
460    ctx.append_activity(
461        "push_recover_worktree",
462        Some(agent_name),
463        model_name,
464        Some(&serde_json::json!({
465            "status": "ok",
466            "worktree": recovered.work_dir.display().to_string(),
467            "branch": recovered.branch,
468        })),
469    )
470    .await;
471
472    Ok(recovered)
473}
474
475/// Push a branch that was just rebased onto its moved remote counterpart.
476///
477/// Separate from the first push attempt so a failure here is reported as orch's own
478/// push failure rather than re-entering the diverged-branch recovery
479async fn push_after_rebase(
480    ctx: &StoreCtx<'_>,
481    work_dir: &Path,
482    branch: &str,
483    default_branch: &str,
484    agent_name: &str,
485    model_name: Option<&str>,
486) -> PushResult {
487    match git_ops::push_branch(work_dir, branch, default_branch).await {
488        Ok(_) => {
489            ctx.append_activity(
490                "push",
491                Some(agent_name),
492                model_name,
493                Some(&serde_json::json!({
494                    "status": "ok",
495                    "branch": branch,
496                    "default_branch": default_branch,
497                    "reason": "pushed_after_rebase",
498                })),
499            )
500            .await;
501            ctx.set(&[
502                ("last_error", serde_json::json!("")),
503                ("push_failures", serde_json::json!(0)),
504            ])
505            .await;
506            PushResult::Success
507        }
508        Err(e) => {
509            tracing::error!(
510                task_id = ctx.task_id,
511                branch = branch,
512                error = ?e,
513                "push failed again after rebasing on the remote branch"
514            );
515            ctx.append_activity(
516                "push",
517                Some(agent_name),
518                model_name,
519                Some(&serde_json::json!({
520                    "status": "error",
521                    "branch": branch,
522                    "default_branch": default_branch,
523                    "reason": "pushed_after_rebase",
524                    "error": e.to_string(),
525                })),
526            )
527            .await;
528            ctx.set(&[(
529                "last_error",
530                serde_json::json!(format!("push failed after rebase on remote branch: {e}")),
531            )])
532            .await;
533            PushResult::Failed
534        }
535    }
536}
537
538async fn push_branch_with_log(
539    ctx: &StoreCtx<'_>,
540    work_dir: &Path,
541    branch: &str,
542    default_branch: &str,
543    agent_name: &str,
544    model_name: Option<&str>,
545) -> PushResult {
546    match git_ops::push_branch(work_dir, branch, default_branch).await {
547        Ok(_) => {
548            ctx.append_activity(
549                "push",
550                Some(agent_name),
551                model_name,
552                Some(&serde_json::json!({
553                    "status": "ok",
554                    "branch": branch,
555                    "default_branch": default_branch,
556                })),
557            )
558            .await;
559            // Clear any stale push failure from a previous run so review_and_merge
560            // does not incorrectly block an approved task.
561            ctx.set(&[
562                ("last_error", serde_json::json!("")),
563                ("push_failures", serde_json::json!(0)),
564            ])
565            .await;
566            PushResult::Success
567        }
568        Err(e) => {
569            let err_str = e.to_string();
570            tracing::error!(task_id = ctx.task_id, error = ?e, "push failed");
571
572            if is_missing_path_error(&e) && !work_dir.exists() {
573                tracing::warn!(
574                    task_id = ctx.task_id,
575                    worktree = %work_dir.display(),
576                    "push failed because worktree path disappeared; will attempt recovery"
577                );
578                ctx.append_activity(
579                    "push",
580                    Some(agent_name),
581                    model_name,
582                    Some(&serde_json::json!({
583                        "status": "retry",
584                        "branch": branch,
585                        "default_branch": default_branch,
586                        "reason": "worktree_missing",
587                        "error": err_str,
588                    })),
589                )
590                .await;
591                return PushResult::WorktreeMissing;
592            }
593
594            // Check if this is a non-fast-forward error that we can recover from.
595            let is_diverged = err_str.contains("non-fast-forward")
596                || err_str.contains("rejected")
597                || err_str.contains("fetch first")
598                || err_str.contains("behind");
599
600            if is_diverged {
601                tracing::info!(
602                    task_id = ctx.task_id,
603                    branch = branch,
604                    "attempting fetch and rebase to recover from diverged branch"
605                );
606
607                // Attempt to fetch and rebase on origin/{branch}.
608                match git_ops::rebase_on_branch(work_dir, branch).await {
609                    Ok(true) => {
610                        // Pushing is orch's step, not the agent's: the commits are already
611                        // written, so push them onto the moved remote branch here. Re-routing
612                        // instead re-ran a finished task until it hit the attempt limit (#3783)
613                        tracing::info!(
614                            task_id = ctx.task_id,
615                            branch = branch,
616                            "rebase succeeded — retrying the push"
617                        );
618                        ctx.append_activity(
619                            "push_rebased",
620                            Some(agent_name),
621                            model_name,
622                            Some(&serde_json::json!({
623                                "status": "rebased",
624                                "branch": branch,
625                                "reason": "diverged_from_remote",
626                            })),
627                        )
628                        .await;
629                        return push_after_rebase(
630                            ctx,
631                            work_dir,
632                            branch,
633                            default_branch,
634                            agent_name,
635                            model_name,
636                        )
637                        .await;
638                    }
639                    Ok(false) => {
640                        // The remote branch already holds every local commit, nothing to push
641                        tracing::info!(
642                            task_id = ctx.task_id,
643                            branch = branch,
644                            "no local commits after fetch — marking as success"
645                        );
646                        ctx.append_activity(
647                            "push",
648                            Some(agent_name),
649                            model_name,
650                            Some(&serde_json::json!({
651                                "status": "ok",
652                                "branch": branch,
653                                "reason": "no_commits_after_fetch",
654                            })),
655                        )
656                        .await;
657                        ctx.set(&[
658                            ("last_error", serde_json::json!("")),
659                            ("push_failures", serde_json::json!(0)),
660                        ])
661                        .await;
662                        return PushResult::Success;
663                    }
664                    Err(rebase_err) => {
665                        // Rebase failed (likely conflicts) — block for human intervention.
666                        tracing::error!(
667                            task_id = ctx.task_id,
668                            branch = branch,
669                            error = ?rebase_err,
670                            "rebase failed — blocking task for human intervention"
671                        );
672                        ctx.append_activity(
673                            "push_rebased",
674                            Some(agent_name),
675                            model_name,
676                            Some(&serde_json::json!({
677                                "status": "error",
678                                "branch": branch,
679                                "error": rebase_err.to_string(),
680                            })),
681                        )
682                        .await;
683                        ctx.set(&[(
684                            "last_error",
685                            serde_json::json!(format!(
686                                "push failed and rebase failed: {rebase_err} — manual resolution required"
687                            )),
688                        )])
689                        .await;
690                        return PushResult::Failed;
691                    }
692                }
693            }
694
695            // Non-recoverable push error — log and return failure.
696            ctx.append_activity(
697                "push",
698                Some(agent_name),
699                model_name,
700                Some(&serde_json::json!({
701                    "status": "error",
702                    "branch": branch,
703                    "default_branch": default_branch,
704                    "error": err_str,
705                })),
706            )
707            .await;
708            ctx.set(&[("last_error", serde_json::json!(format!("push failed: {e}")))])
709                .await;
710            PushResult::Failed
711        }
712    }
713}
714
715/// True when the GitHub client refused the call locally because its rate-limit window is active.
716fn is_local_rate_limit_skip(err_str: &str) -> bool {
717    err_str.contains("rate-limited, skipping call")
718}
719
720/// Create (or find an existing) PR and log the activity.
721///
722/// Returns `(has_pr, has_pushed, pr_base_invalid)`. `has_pushed` may be set to
723/// `false` when a 422 "no commits" GitHub error indicates the branch was
724/// already merged. `pr_base_invalid` is set when the base branch is rejected
725/// by GitHub (even after the API fallback), which is a terminal condition.
726async fn create_pr_with_log(
727    ctx: &StoreCtx<'_>,
728    wt: &worktree::WorktreeSetup,
729    task_title: &str,
730    resp: &AgentResponse,
731    agent_name: &str,
732    model_name: Option<&str>,
733    mut has_pushed: bool,
734) -> (bool, bool, bool) {
735    match git_ops::create_pr_if_needed(
736        &wt.work_dir,
737        &wt.branch,
738        task_title,
739        &resp.summary,
740        &resp.accomplished,
741        &resp.remaining,
742        &resp.files,
743        ctx.task_id,
744        agent_name,
745        model_name,
746        ctx.repo,
747        &wt.default_branch,
748    )
749    .await
750    {
751        Ok(ref url) => {
752            // Save pr_number to the store so the review gate can find it
753            // immediately without racing GitHub's list-API cache (~300 ms lag).
754            // This is set for both newly-created and pre-existing PRs.
755            match review::parse_pr_number_from_url(url) {
756                Ok(pr_num) => {
757                    if let Err(e) = store::store_set_result(
758                        ctx.store,
759                        ctx.repo,
760                        ctx.task_id,
761                        &[("pr_number", serde_json::json!(pr_num))],
762                    )
763                    .await
764                    {
765                        tracing::error!(
766                            task_id = ctx.task_id,
767                            pr_url = %url,
768                            err = %e,
769                            "CRITICAL: failed to save pr_number to store — downstream review gate may trigger duplicate gh pr create"
770                        );
771                    }
772                }
773                Err(e) => {
774                    tracing::warn!(
775                        task_id = ctx.task_id,
776                        pr_url = %url,
777                        error = %e,
778                        "PR created but failed to parse PR number from URL"
779                    );
780                }
781            }
782            ctx.append_activity(
783                "pr_create",
784                Some(agent_name),
785                model_name,
786                Some(&serde_json::json!({"status": "created", "url": url})),
787            )
788            .await;
789            (true, has_pushed, false)
790        }
791        Err(e) => {
792            let err_str = format!("{e}");
793            tracing::error!(task_id = ctx.task_id, error = ?e, "create PR failed");
794            ctx.append_activity(
795                "pr_create",
796                Some(agent_name),
797                model_name,
798                Some(&serde_json::json!({
799                    "status": "error",
800                    "branch": wt.branch,
801                    "error": err_str.clone(),
802                })),
803            )
804            .await;
805            ctx.set(&[(
806                "last_error",
807                serde_json::json!(format!("create PR failed: {e}")),
808            )])
809            .await;
810            // Distinguish between terminal 422 errors and recoverable ones.
811            // - "No commits between" / "head invalid" → agent made no code changes
812            //   or the branch was already merged.  Clearing has_pushed lets the
813            //   task fall through to the "done" path.
814            // - "base invalid" → the base branch name is wrong.  After the
815            //   GitHub API fallback in create_pr_if_needed also fails, this is a
816            //   terminal configuration error.  We set pr_base_invalid so the
817            //   classifier blocks the task instead of re-dispatching forever.
818            // A GitHub auth error is terminal too: the commits are pushed, so only a token fix helps
819            let mut pr_base_invalid = false;
820            if is_local_rate_limit_skip(&err_str) {
821                // No request was sent and the branch is pushed. Re-running the agent cannot help,
822                // so report the PR as pending and let the review gate create it once calls resume
823                tracing::warn!(
824                    task_id = ctx.task_id,
825                    "PR creation skipped by local GitHub rate limit — review gate will create the PR"
826                );
827                return (true, has_pushed, false);
828            }
829            if agents::patterns::detect_auth_error(&err_str).is_some() {
830                tracing::error!(
831                    task_id = ctx.task_id,
832                    "PR creation failed with GitHub auth error — blocking task"
833                );
834                pr_base_invalid = true;
835            } else if err_str.contains("422") {
836                if err_str.contains("No commits between") || err_str.contains("head") {
837                    tracing::info!(
838                        task_id = ctx.task_id,
839                        "PR creation returned 422/no-commits/head-invalid — agent made no code changes or branch merged, marking done"
840                    );
841                    has_pushed = false;
842                } else if git_ops::is_invalid_base_error(&err_str) {
843                    tracing::error!(
844                        task_id = ctx.task_id,
845                        "PR creation failed with invalid base branch — blocking task"
846                    );
847                    pr_base_invalid = true;
848                }
849            }
850            (false, has_pushed, pr_base_invalid)
851        }
852    }
853}
854
855/// Run the full auto-commit → push → PR pipeline.
856async fn run_git_ops(
857    ctx: &StoreCtx<'_>,
858    wt: &worktree::WorktreeSetup,
859    task_title: &str,
860    resp: &AgentResponse,
861    agent_name: &str,
862    model_name: Option<&str>,
863    new_attempts: u32,
864) -> GitOpsResult {
865    auto_commit_changes(ctx, &wt.work_dir, task_title, agent_name, new_attempts).await;
866
867    let mut has_commits =
868        check_commits_and_clear_stale_errors(ctx, &wt.work_dir, &wt.default_branch).await;
869
870    // Skip push + PR if there are no commits ahead of the default branch.
871    // No-op tasks (e.g. "nothing to execute") produce no commits, so pushing
872    // and creating a PR would just waste API calls and trigger 422 errors.
873    let push_result = if has_commits {
874        push_branch_with_log(
875            ctx,
876            &wt.work_dir,
877            &wt.branch,
878            &wt.default_branch,
879            agent_name,
880            model_name,
881        )
882        .await
883    } else {
884        store::store_log_activity(
885            ctx.store,
886            ctx.repo,
887            ctx.task_id,
888            "push",
889            None,
890            None,
891            Some(agent_name),
892            model_name,
893            Some(&serde_json::json!({
894                "status": "skipped",
895                "reason": "no_commits_ahead",
896                "branch": wt.branch,
897                "default_branch": wt.default_branch,
898            })),
899        )
900        .await;
901        PushResult::NoCommits
902    };
903    let push_result = if matches!(push_result, PushResult::WorktreeMissing) {
904        match recover_missing_worktree_for_push(ctx, wt, task_title, agent_name, model_name).await {
905            Ok(recovered) => {
906                let wt_for_ops = &recovered;
907                has_commits = check_commits_and_clear_stale_errors(
908                    ctx,
909                    &wt_for_ops.work_dir,
910                    &wt_for_ops.default_branch,
911                )
912                .await;
913                if has_commits {
914                    push_branch_with_log(
915                        ctx,
916                        &wt_for_ops.work_dir,
917                        &wt_for_ops.branch,
918                        &wt_for_ops.default_branch,
919                        agent_name,
920                        model_name,
921                    )
922                    .await
923                } else {
924                    PushResult::NoCommits
925                }
926            }
927            Err(e) => {
928                tracing::error!(
929                    task_id = ctx.task_id,
930                    err = %e,
931                    "worktree recovery before push failed"
932                );
933                ctx.append_activity(
934                    "push_recover_worktree",
935                    Some(agent_name),
936                    model_name,
937                    Some(&serde_json::json!({
938                        "status": "error",
939                        "worktree": wt.work_dir.display().to_string(),
940                        "branch": wt.branch,
941                        "error": e.to_string(),
942                    })),
943                )
944                .await;
945                ctx.set(&[(
946                    "last_error",
947                    serde_json::json!(format!(
948                        "push failed: worktree disappeared and recovery failed: {e}"
949                    )),
950                )])
951                .await;
952                PushResult::Failed
953            }
954        }
955    } else {
956        push_result
957    };
958
959    let mut has_pushed = matches!(push_result, PushResult::Success);
960    let mut has_pr = false;
961
962    // Create PR (skip if push failed or repo is unknown)
963    if has_commits && matches!(push_result, PushResult::Failed) {
964        tracing::warn!(
965            task_id = ctx.task_id,
966            "skipping PR creation due to push failure"
967        );
968    } else if matches!(push_result, PushResult::NoCommits) {
969        tracing::info!(
970            task_id = ctx.task_id,
971            "no commits ahead of default branch, skipping push + PR"
972        );
973    } else if !has_pushed {
974        // no commits — already logged "no commits ahead, skipping push + PR" at INFO level above
975    } else if ctx.repo.is_empty() {
976        tracing::warn!(
977            task_id = ctx.task_id,
978            "skipping PR creation — repo is empty (internal task?)"
979        );
980    } else {
981        let (pr, updated_pushed, pr_base_invalid) = create_pr_with_log(
982            ctx, wt, task_title, resp, agent_name, model_name, has_pushed,
983        )
984        .await;
985        has_pr = pr;
986        has_pushed = updated_pushed;
987        return GitOpsResult {
988            has_pr,
989            has_pushed,
990            has_commits,
991            pr_base_invalid,
992        };
993    }
994
995    GitOpsResult {
996        has_pr,
997        has_pushed,
998        has_commits,
999        pr_base_invalid: false,
1000    }
1001}
1002
1003// ── Private status-computation helpers ───────────────────────────────────────
1004
1005/// Comment on and close a PR whose change already landed elsewhere.
1006async fn close_superseded_pr(ctx: &StoreCtx<'_>, pr_number: u64) {
1007    if ctx.repo.is_empty() {
1008        return;
1009    }
1010    match GhHttp::new() {
1011        Ok(gh) => {
1012            let note = "Closing as superseded: the change is already on the default branch, so this branch has no commits ahead of it.";
1013            if let Err(e) = gh.add_comment(ctx.repo, &pr_number.to_string(), note).await {
1014                tracing::warn!(task_id = ctx.task_id, pr_number, err = %e, "failed to comment on superseded PR");
1015            }
1016            if let Err(e) = gh.close_pr(ctx.repo, pr_number).await {
1017                tracing::warn!(task_id = ctx.task_id, pr_number, err = %e, "failed to close superseded PR");
1018            }
1019        }
1020        Err(e) => {
1021            tracing::warn!(task_id = ctx.task_id, err = %e, "cannot close superseded PR");
1022        }
1023    }
1024    // Dropping the PR lets the blocker refresh read the closed, completed issue with no PR
1025    ctx.set(&[
1026        (
1027            "last_error",
1028            serde_json::json!(
1029                "PR closed as superseded: the fix already landed on the default branch"
1030            ),
1031        ),
1032        ("pr_number", serde_json::json!(null)),
1033    ])
1034    .await;
1035}
1036
1037/// The PR to close when a merge-conflict reroute produced no commits.
1038///
1039/// `merge_conflict_retries > 0` means review_poll sent the task back to resolve a
1040/// conflict. Coming back with nothing ahead of the default branch means another
1041/// PR already landed the same fix, so the open PR is superseded (#3780)
1042fn superseded_conflict_pr(
1043    merge_conflict_retries: i32,
1044    pr_number: Option<i32>,
1045    has_commits: bool,
1046) -> Option<u64> {
1047    if has_commits || merge_conflict_retries <= 0 {
1048        return None;
1049    }
1050    pr_number
1051        .filter(|n| *n > 0)
1052        .and_then(|n| u64::try_from(n).ok())
1053}
1054
1055/// Read push-failure state from the store and increment `push_failures` if needed.
1056async fn detect_push_failure_state(
1057    ctx: &StoreCtx<'_>,
1058    has_commits: bool,
1059    has_pushed: bool,
1060) -> PushFailureState {
1061    // Load stored task once to inspect last_error (avoid repeated DB reads).
1062    let stored_last_error = ctx
1063        .get_task()
1064        .await
1065        .map(|t| t.last_error)
1066        .unwrap_or_default();
1067
1068    let push_failed = !has_pushed && has_commits && stored_last_error.contains("push failed");
1069
1070    // Detect workflow scope errors — these are non-retryable token permission issues.
1071    // Rerouting to a different agent won't help since the same token is used.
1072    let is_workflow_scope_failure = push_failed
1073        && (stored_last_error.contains("workflow` scope")
1074            || stored_last_error.contains("workflow' scope")
1075            || (stored_last_error.contains("refusing to allow")
1076                && stored_last_error.contains("workflow")));
1077
1078    let is_rebase_conflict_failure =
1079        push_failed && stored_last_error.contains("push failed and rebase failed:");
1080
1081    // Track push failures — block after 3 consecutive failures.
1082    // Workflow-scope failures are non-retryable and blocked immediately; do not
1083    // increment this retry counter for them so that later normal push failures
1084    // are not prematurely blocked.
1085    let push_failures = if push_failed && !is_workflow_scope_failure && !is_rebase_conflict_failure
1086    {
1087        match ctx.increment("push_failures").await {
1088            Ok(v) => v,
1089            Err(e) => {
1090                tracing::warn!(
1091                    task_id = ctx.task_id,
1092                    err = %e,
1093                    "failed to increment push_failures — reading current value from store"
1094                );
1095                // On DB error, read current value to ensure blocking threshold is still reachable
1096                ctx.get_task()
1097                    .await
1098                    .map_or(1, |t| u64::try_from(t.push_failures).unwrap_or(0) + 1)
1099                // assume at least 1 if we can't read either
1100            }
1101        }
1102    } else {
1103        0
1104    };
1105
1106    PushFailureState {
1107        push_failed,
1108        is_workflow_scope_failure,
1109        is_rebase_conflict_failure,
1110        push_failures,
1111        stored_last_error,
1112    }
1113}
1114
1115/// Increment `no_code_reroutes` and log a warning when the agent completed
1116/// without producing code changes on an external task requiring a PR.
1117async fn detect_no_code_reroutes(
1118    ctx: &StoreCtx<'_>,
1119    is_no_code_reroute: bool,
1120    new_attempts: u32,
1121    max_reroutes: u32,
1122) -> u64 {
1123    if !is_no_code_reroute {
1124        return 0;
1125    }
1126    tracing::warn!(
1127        task_id = ctx.task_id,
1128        attempts = new_attempts,
1129        max_reroutes,
1130        "agent reported done but produced no code changes on external task requiring PR"
1131    );
1132    match ctx.increment("no_code_reroutes").await {
1133        Ok(v) => v,
1134        Err(e) => {
1135            tracing::warn!(
1136                task_id = ctx.task_id,
1137                err = %e,
1138                "failed to increment no_code_reroutes — reading current value from store"
1139            );
1140            // On DB error, read current value to ensure blocking threshold is still reachable
1141            ctx.get_task()
1142                .await
1143                .map_or(1, |t| u64::try_from(t.no_code_reroutes).unwrap_or(0) + 1)
1144            // assume at least 1 if we can't read either
1145        }
1146    }
1147}
1148
1149// ── Post-decision side-effect helpers ─────────────────────────────────────────
1150
1151/// Build a human-readable reason string from a "blocked" agent response.
1152///
1153/// Priority: explicit `error` field → `summary` → fallback.
1154/// Appends `remaining` items if present so operators know what was left to do.
1155fn agent_blocked_reason(resp: &AgentResponse) -> String {
1156    let summary_opt = if resp.summary.is_empty() {
1157        None
1158    } else {
1159        Some(resp.summary.as_str())
1160    };
1161    let base = resp
1162        .error
1163        .as_deref()
1164        .filter(|s| !s.is_empty())
1165        .or(summary_opt)
1166        .unwrap_or("agent returned blocked status without a reason");
1167    if resp.remaining.is_empty() {
1168        base.to_string()
1169    } else {
1170        format!("{base}. Remaining: {}", resp.remaining.join("; "))
1171    }
1172}
1173
1174/// True when a blocked response names a denied `git push` and nothing else as the blocker.
1175fn is_denied_push_block(resp: &AgentResponse) -> bool {
1176    let r = agent_blocked_reason(resp).to_lowercase();
1177    r.contains("push")
1178        && [
1179            "denied",
1180            "not allowed",
1181            "permission",
1182            "disallowed",
1183            "approval",
1184        ]
1185        .iter()
1186        .any(|p| r.contains(p))
1187}
1188
1189/// Heuristic classifier for agent-returned blocked reasons.
1190///
1191/// Returns true when the reason looks transient/retryable (network/provider/
1192/// auth-temporary/environment availability issues), and false for likely
1193/// permanent/input/configuration issues.
1194fn is_retryable_blocked_reason(reason: &str) -> bool {
1195    let r = reason.to_lowercase();
1196
1197    let permanent_patterns = [
1198        "not found",
1199        "does not exist",
1200        "invalid",
1201        "malformed",
1202        "syntax error",
1203        "unsupported",
1204        "unimplemented",
1205        "manual intervention required",
1206        "human intervention required",
1207    ];
1208    if permanent_patterns.iter().any(|p| r.contains(p)) {
1209        return false;
1210    }
1211
1212    let transient_patterns = [
1213        "unavailable",
1214        "temporarily unavailable",
1215        "timeout",
1216        "timed out",
1217        "connection reset",
1218        "connection refused",
1219        "network error",
1220        "rate limit",
1221        "try again",
1222        "service down",
1223        "permission denied",
1224        "worktree lock",
1225        "git lock",
1226        "lockfile permission",
1227        "lock permission",
1228        "resource busy",
1229        "could not resolve host",
1230        // Credential providers in headless/tmux sessions can fail transiently
1231        // when GPG agent/passphrase is unavailable.
1232        "gpg decryption fails",
1233        "gpg decryption failed",
1234        "gpg agent",
1235        "no passphrase available",
1236        "passphrase unavailable",
1237        "pass store",
1238        "passwordstoreprovider",
1239        "agentvaultprovider",
1240        "onepasswordprovider",
1241        "decryption failed in this agent session",
1242    ];
1243    transient_patterns.iter().any(|p| r.contains(p))
1244}
1245
1246/// Apply tracing and store writes that depend on the final status decision.
1247#[expect(
1248    clippy::too_many_arguments,
1249    reason = "the decision and its context arrive as separate values"
1250)]
1251async fn apply_post_decision_effects(
1252    ctx: &StoreCtx<'_>,
1253    final_status: &str,
1254    resp_status: &str,
1255    push_state: &PushFailureState,
1256    is_no_code_reroute: bool,
1257    has_pr: bool,
1258    has_pushed: bool,
1259    is_pr_base_invalid: bool,
1260    has_delegations: bool,
1261    is_external: bool,
1262    no_code_reroutes: u64,
1263    max_reroutes: u32,
1264    agent_name: &str,
1265) {
1266    if push_state.is_workflow_scope_failure {
1267        // Non-retryable — block immediately with actionable guidance.
1268        tracing::error!(
1269            task_id = ctx.task_id,
1270            "push failed: token lacks `workflow` OAuth scope — blocking immediately \
1271             (rerouting would not help)"
1272        );
1273        ctx.set(&[(
1274            "last_error",
1275            serde_json::json!(format!(
1276                "push failed: GitHub token lacks `workflow` OAuth scope. \
1277                 The agent modified .github/workflows/ files but the token cannot push them. \
1278                 Fix: add `workflow` scope to your GitHub token, or use a GitHub App for auth. \
1279                 Original error: {}",
1280                push_state.stored_last_error
1281            )),
1282        )])
1283        .await;
1284    } else if push_state.is_rebase_conflict_failure {
1285        tracing::error!(
1286            task_id = ctx.task_id,
1287            "push failed and automatic rebase recovery failed — blocking for human intervention"
1288        );
1289        ctx.set(&[
1290            ("agent", serde_json::json!(null)),
1291            ("model", serde_json::json!(null)),
1292        ])
1293        .await;
1294    } else if push_state.push_failed {
1295        // Clear agent and model so router picks a different one on reroute (#1604)
1296        ctx.set(&[
1297            ("agent", serde_json::json!(null)),
1298            ("model", serde_json::json!(null)),
1299        ])
1300        .await;
1301        let push_failures = push_state.push_failures;
1302        if push_failures >= 3 {
1303            tracing::error!(
1304                task_id = ctx.task_id,
1305                push_failures,
1306                "push failed {push_failures} times — blocking for human intervention"
1307            );
1308        } else {
1309            tracing::warn!(
1310                task_id = ctx.task_id,
1311                push_failures,
1312                "agent done but push failed ({push_failures}/3) — rerouting to different agent"
1313            );
1314        }
1315    } else if resp_status == "done" && !has_pr && has_delegations {
1316        tracing::info!(
1317            task_id = ctx.task_id,
1318            "agent reported done with delegations but no PR — setting blocked"
1319        );
1320    } else if (resp_status == "done" || resp_status == "completed")
1321        && !has_pr
1322        && has_pushed
1323        && is_pr_base_invalid
1324    {
1325        tracing::error!(
1326            task_id = ctx.task_id,
1327            "PR creation failed with invalid base branch — blocking for human intervention"
1328        );
1329        ctx.set(&[(
1330            "last_error",
1331            serde_json::json!(
1332                "PR creation failed terminally (invalid base branch or GitHub auth error). The agent's commits were pushed successfully. \
1333                 Check the repository default branch and the GitHub token, then create the PR manually or unblock the task."
1334            ),
1335        )])
1336        .await;
1337    } else if resp_status == "done" && !has_pr && has_pushed {
1338        // Push succeeded but PR creation failed after retries.
1339        tracing::warn!(
1340            task_id = ctx.task_id,
1341            "agent done, commits pushed, but PR creation failed — re-dispatching as routed"
1342        );
1343    } else if is_no_code_reroute {
1344        // Record which agent produced this no-code result so the same-agent
1345        // detection on the next run can prevent loop-backs (#2410, #2686).
1346        // Also clear agent/model to force re-routing.
1347
1348        if final_status == "blocked" {
1349            // final_status is "blocked" either because max_reroutes was exhausted
1350            // or because is_same_agent was detected (both cases are handled by
1351            // classify_final_status). Log accordingly.
1352            let prev_no_code_agent = ctx
1353                .get_task()
1354                .await
1355                .map(|t| t.no_code_last_agent)
1356                .unwrap_or_default();
1357            let is_same_agent = !prev_no_code_agent.is_empty() && prev_no_code_agent == agent_name;
1358
1359            let msg = if is_same_agent {
1360                tracing::error!(
1361                    task_id = ctx.task_id,
1362                    agent = %agent_name,
1363                    "blocking same-agent no-code loop: agent {} would be selected again",
1364                    agent_name
1365                );
1366                format!(
1367                    "agent {agent_name} completed without code changes twice — same-agent loop detected, blocking for human review"
1368                )
1369            } else {
1370                tracing::error!(
1371                    task_id = ctx.task_id,
1372                    no_code_reroutes,
1373                    max_reroutes,
1374                    "blocking no-code reroute: reached max reroute attempts ({}/{}) for no-code-result",
1375                    no_code_reroutes,
1376                    max_reroutes
1377                );
1378                format!(
1379                    "agent completed without code changes after {no_code_reroutes}/{max_reroutes} reroute attempts on external task requiring PR"
1380                )
1381            };
1382            ctx.set(&[("last_error", serde_json::json!(msg))]).await;
1383        } else {
1384            // Rerouting — no blocking error to record yet.
1385            ctx.set(&[(
1386                "last_error",
1387                serde_json::json!(
1388                    "agent completed without code changes on external task requiring PR"
1389                ),
1390            )])
1391            .await;
1392        }
1393
1394        // Always record which agent produced this no-code result so the router can
1395        // skip it on the next attempt. Also clear agent/model to force re-routing
1396        // (the router will pick a fresh agent, which will be caught by the same-agent
1397        // check on the next round if it's the same agent).
1398        ctx.set(&[
1399            ("no_code_last_agent", serde_json::json!(agent_name)),
1400            ("agent", serde_json::json!(null)),
1401            ("model", serde_json::json!(null)),
1402        ])
1403        .await;
1404    } else if resp_status == "done" && !has_pr && is_external {
1405        // External task with non-code labels (e.g. documentation, research) —
1406        // allowed to be marked done without a PR.
1407        tracing::info!(
1408            task_id = ctx.task_id,
1409            "external task with non-code labels reported done — marking done without PR"
1410        );
1411    } else if resp_status == "done" && !has_pr {
1412        tracing::info!(
1413            task_id = ctx.task_id,
1414            "internal task reported done with no PR — marking done"
1415        );
1416    }
1417}
1418
1419/// Store token usage if both input and output token counts are known.
1420async fn store_token_usage(
1421    ctx: &StoreCtx<'_>,
1422    input_tokens: Option<u64>,
1423    output_tokens: Option<u64>,
1424    model_name: Option<&str>,
1425) {
1426    let (Some(input), Some(output)) = (input_tokens, output_tokens) else {
1427        return;
1428    };
1429    let model = model_name.unwrap_or("haiku");
1430    if let Some(st) = ctx.store {
1431        if let Some(store_id) = ctx.store_id_opt {
1432            if let Err(e) = st.store_tokens(store_id, input, output, model).await {
1433                tracing::warn!(task_id = ctx.task_id, ?e, "failed to store token usage");
1434            }
1435        } else if let Ok(Some(store_id)) = st.resolve_task_id(ctx.repo, ctx.task_id).await
1436            && let Err(e) = st.store_tokens(store_id, input, output, model).await
1437        {
1438            tracing::warn!(task_id = ctx.task_id, ?e, "failed to store token usage");
1439        }
1440    }
1441}
1442
1443/// Handle a successful agent response: commit, push, PR, delegations, tokens.
1444///
1445/// Returns `Ok((status, push_failed))` where `status` is the final task status
1446/// string and `push_failed` is `true` when a push was attempted but failed (for
1447/// audit trail classification).
1448#[expect(
1449    clippy::too_many_arguments,
1450    reason = "the runner passes each part of the run result"
1451)]
1452pub async fn handle_success(
1453    task_id: &str,
1454    parsed: agents::ParsedResponse,
1455    wt: &worktree::WorktreeSetup,
1456    task_title: &str,
1457    agent_name: &str,
1458    model_name: Option<&str>,
1459    new_attempts: u32,
1460    repo: &str,
1461    store: &Option<Arc<TaskStore>>,
1462    raw_stdout: &str,
1463) -> anyhow::Result<(String, bool)> {
1464    // Extract token counts before consuming parsed.
1465    let input_tokens = parsed.input_tokens;
1466    let output_tokens = parsed.output_tokens;
1467    let mut resp = parsed.response;
1468
1469    tracing::info!(
1470        task_id,
1471        status = resp.status,
1472        summary = resp.summary,
1473        "agent completed successfully"
1474    );
1475
1476    let push_denied = resp.status == "blocked" && is_denied_push_block(&resp);
1477
1478    // Resolve numeric store_id once so we can reuse it for multiple store ops
1479    // in this hot path and avoid repeated external_id -> store_id SQL lookups.
1480    let store_id_opt = store::resolve_store_id(store, repo, task_id).await;
1481    let ctx = StoreCtx {
1482        store,
1483        store_id_opt,
1484        repo,
1485        task_id,
1486    };
1487
1488    ctx.set(&[("network_retries", serde_json::json!(0))]).await;
1489
1490    // Auto-commit, push, create PR
1491    let git = if resp.status == "done"
1492        || resp.status == "completed"
1493        || resp.status == "in_progress"
1494        || resp.status == "needs_review"
1495        || push_denied
1496    {
1497        run_git_ops(
1498            &ctx,
1499            wt,
1500            task_title,
1501            &resp,
1502            agent_name,
1503            model_name,
1504            new_attempts,
1505        )
1506        .await
1507    } else {
1508        GitOpsResult::default()
1509    };
1510
1511    // The agent's only blocker was the denied push and orch pushed for it, so
1512    // the task continues as if the agent had returned done
1513    if push_denied && (git.has_pr || git.has_pushed) {
1514        tracing::info!(
1515            task_id,
1516            "agent blocked on denied push, orch pushed the branch"
1517        );
1518        resp.status = "done".to_string();
1519    }
1520
1521    // Store delegations in store if present (processed by run_with_context)
1522    if !resp.delegations.is_empty() {
1523        ctx.set(&[("delegations", serde_json::json!(resp.delegations))])
1524            .await;
1525    }
1526
1527    // Store result in task store
1528    // If agent said "done" and a PR exists, send to review before merge.
1529    // If agent said "done", pushed commits, but PR creation failed — review gate creates PR.
1530    // If agent said "done", no PR, and no delegations — work is complete
1531    // (e.g., review/analysis jobs that create issues but no code changes).
1532    // If agent said "done", no PR, but has delegations — blocked on children.
1533    let has_delegations = !resp.delegations.is_empty();
1534
1535    // Detect push failure, increment counters.
1536    let push_state = detect_push_failure_state(&ctx, git.has_commits, git.has_pushed).await;
1537
1538    // Determine whether this external task can be marked done without a PR.
1539    // External tasks always require a PR before reaching `done` — unless
1540    // commits were successfully pushed (in which case the `has_pushed` branch
1541    // handles routing). The `is_non_code_task` heuristic was removed because
1542    // it relied on agent output and could be fooled: an agent claiming
1543    // "already implemented" or "config-only" would match non-code keywords
1544    // and close the issue without verification.
1545    let is_external = !task_id.starts_with("internal:");
1546    let requires_pr = is_external && !git.has_pr && !resp.status.starts_with("needs_review");
1547
1548    // Preferred config key `workflow.max_reroute_attempts`; fall back to
1549    // `workflow.max_attempts` for backwards compatibility.
1550    let max_reroutes: u32 = config::get("workflow.max_reroute_attempts")
1551        .ok()
1552        .and_then(|s| s.parse().ok())
1553        .or_else(|| {
1554            config::get("workflow.max_attempts")
1555                .ok()
1556                .and_then(|s| s.parse().ok())
1557        })
1558        .unwrap_or(3);
1559
1560    // Determine whether we are entering the no-code-reroute branch (all
1561    // earlier chain conditions are false + done + requires_pr).
1562    let is_no_code_reroute = !push_state.is_workflow_scope_failure
1563        && !push_state.push_failed
1564        && (resp.status == "done" || resp.status == "completed")
1565        && !has_delegations
1566        && !git.has_pushed
1567        && requires_pr;
1568
1569    // A conflict reroute with nothing ahead of the default branch is a landed
1570    // fix, not a no-code run: close the superseded PR and mark the task done.
1571    let superseded_pr = match (is_no_code_reroute, ctx.get_task().await) {
1572        (true, Some(task)) => {
1573            superseded_conflict_pr(task.merge_conflict_retries, task.pr_number, git.has_commits)
1574        }
1575        _ => None,
1576    };
1577    let is_no_code_reroute = is_no_code_reroute && superseded_pr.is_none();
1578
1579    let no_code_reroutes =
1580        detect_no_code_reroutes(&ctx, is_no_code_reroute, new_attempts, max_reroutes).await;
1581
1582    let blocked_reason = if resp.status == "blocked" {
1583        Some(agent_blocked_reason(&resp))
1584    } else {
1585        None
1586    };
1587    let is_retryable_blocked = blocked_reason
1588        .as_deref()
1589        .is_some_and(is_retryable_blocked_reason);
1590
1591    // ── Detect same-agent loop ───────────────────────────────────────────────
1592    //
1593    // If this is a no-code reroute and the agent is the same as the one that
1594    // produced the previous no-code result, flag it for blocking (#2410, #2686).
1595    let is_same_agent = if is_no_code_reroute {
1596        let prev_no_code_agent = ctx
1597            .get_task()
1598            .await
1599            .map(|t| t.no_code_last_agent)
1600            .unwrap_or_default();
1601        !prev_no_code_agent.is_empty() && prev_no_code_agent == agent_name
1602    } else {
1603        false
1604    };
1605
1606    // ── Pure status decision ─────────────────────────────────────────────────
1607
1608    let final_status_owned = classify_final_status(&DecisionInput {
1609        agent_status: &resp.status,
1610        is_workflow_scope_failure: push_state.is_workflow_scope_failure,
1611        is_rebase_conflict_failure: push_state.is_rebase_conflict_failure,
1612        push_failed: push_state.push_failed,
1613        push_failures: push_state.push_failures,
1614        has_pr: git.has_pr,
1615        has_delegations,
1616        has_pushed: git.has_pushed,
1617        is_pr_base_invalid: git.pr_base_invalid,
1618        requires_pr,
1619        no_code_reroutes,
1620        max_reroutes,
1621        is_same_agent,
1622        is_completed_status: resp.status == "completed",
1623        is_retryable_blocked,
1624        is_superseded_conflict: superseded_pr.is_some(),
1625    });
1626    let final_status = final_status_owned.as_str();
1627
1628    // ── Post-decision side effects (tracing + store updates) ─────────────────
1629
1630    apply_post_decision_effects(
1631        &ctx,
1632        final_status,
1633        &resp.status,
1634        &push_state,
1635        is_no_code_reroute,
1636        git.has_pr,
1637        git.has_pushed,
1638        git.pr_base_invalid,
1639        has_delegations,
1640        is_external,
1641        no_code_reroutes,
1642        max_reroutes,
1643        agent_name,
1644    )
1645    .await;
1646
1647    // When the agent explicitly returned "blocked", persist its explanation into
1648    // last_error so `orch task get` surfaces the reason without requiring manual
1649    // inspection of task_runs.parsed_response.  The apply_post_decision_effects
1650    // branches above only fire on push/no-code scenarios; a raw agent-blocked
1651    // response falls through all of them and would leave last_error empty.
1652    if resp.status == "blocked" {
1653        let reason = blocked_reason.unwrap_or_else(|| agent_blocked_reason(&resp));
1654        if final_status == "new" {
1655            // Retryable external block: cooldown failed agent/model, clear selection,
1656            // and reroute after backoff instead of permanently blocking.
1657            cooldown::record_agent_failure_with_message(agent_name, &reason).await;
1658            if let Some(m) = model_name {
1659                cooldown::record_model_failure(agent_name, m).await;
1660            }
1661            ctx.set(&[
1662                (
1663                    "last_error",
1664                    serde_json::json!(format!(
1665                        "transient blocker (auto-retry scheduled): {reason}"
1666                    )),
1667                ),
1668                ("agent", serde_json::json!(null)),
1669                ("model", serde_json::json!(null)),
1670            ])
1671            .await;
1672            tracing::warn!(
1673                task_id = ctx.task_id,
1674                reason = %reason,
1675                "agent returned retryable blocked status — rerouting as new with cooldown"
1676            );
1677        } else if final_status == "blocked" {
1678            ctx.set(&[("last_error", serde_json::json!(reason))]).await;
1679        }
1680    }
1681
1682    if let Some(pr_number) = superseded_pr {
1683        tracing::info!(
1684            task_id = ctx.task_id,
1685            pr_number,
1686            "conflict reroute found the fix already on the default branch — closing superseded PR"
1687        );
1688        close_superseded_pr(&ctx, pr_number).await;
1689    }
1690
1691    ctx.set(&[("summary", serde_json::json!(resp.summary))])
1692        .await;
1693
1694    // Store token usage — prefer agent-parsed tokens, fall back to response
1695    store_token_usage(
1696        &ctx,
1697        input_tokens.or(resp.input_tokens),
1698        output_tokens.or(resp.output_tokens),
1699        model_name,
1700    )
1701    .await;
1702
1703    // Store learnings for memory (for future retries)
1704    response::store_learnings_from_response(
1705        task_id,
1706        new_attempts,
1707        agent_name,
1708        model_name,
1709        &resp,
1710        resp.error.as_deref(),
1711        store,
1712        repo,
1713        raw_stdout,
1714    )
1715    .await;
1716
1717    // Note: done → in_review transition is handled by the engine
1718    // after triggering the review agent (engine/mod.rs)
1719    Ok((final_status.to_string(), push_state.push_failed))
1720}
1721
1722/// Labels that indicate a task is non-code and can be marked done without a PR.
1723#[cfg(test)]
1724mod tests {
1725    use std::fs as std_fs;
1726    use std::process::Command;
1727
1728    use super::*;
1729    use crate::store::NewTask;
1730
1731    #[test]
1732    fn local_rate_limit_skip_is_detected() {
1733        assert!(is_local_rate_limit_skip(
1734            "GitHub API error: GitHub REST API rate-limited, skipping call for 236s"
1735        ));
1736        assert!(!is_local_rate_limit_skip(
1737            "GitHub API POST failed (502): Bad Gateway"
1738        ));
1739    }
1740
1741    /// Helper: run git in `dir`, panicking on failure
1742    fn git(dir: &Path, args: &[&str]) {
1743        let out = Command::new("git")
1744            .current_dir(dir)
1745            .args(args)
1746            .output()
1747            .unwrap_or_else(|e| panic!("git {args:?} failed to start: {e}"));
1748        assert!(
1749            out.status.success(),
1750            "git {args:?} failed:\nstdout: {}\nstderr: {}",
1751            String::from_utf8_lossy(&out.stdout),
1752            String::from_utf8_lossy(&out.stderr)
1753        );
1754    }
1755
1756    fn init_clone(remote: &Path, dir: &Path) {
1757        git(dir, &["clone", remote.to_str().unwrap(), "."]);
1758        git(dir, &["config", "user.email", "t@t.com"]);
1759        git(dir, &["config", "user.name", "T"]);
1760        git(dir, &["config", "commit.gpgsign", "false"]);
1761    }
1762
1763    /**
1764     * Someone rebased the PR branch from another clone and force-pushed it, so orch's
1765     * push is rejected with a stale lease. Pushing is orch's own step: it must rebase
1766     * onto the moved branch and push again, never hand finished work back to the agent
1767     * as a fresh attempt (#3783)
1768     **/
1769    #[tokio::test]
1770    async fn push_rejected_after_remote_rebase_is_pushed_not_rerouted() {
1771        let remote = tempfile::tempdir().unwrap();
1772        git(remote.path(), &["init", "--bare", "-b", "main"]);
1773
1774        // orch's worktree: main, then a feature branch with the task's commit
1775        let work = tempfile::tempdir().unwrap();
1776        init_clone(remote.path(), work.path());
1777        std_fs::write(work.path().join("README.md"), "base").unwrap();
1778        git(work.path(), &["add", "."]);
1779        git(work.path(), &["commit", "-m", "init"]);
1780        git(work.path(), &["push", "origin", "HEAD:main"]);
1781        git(work.path(), &["checkout", "-b", "feat"]);
1782        std_fs::write(work.path().join("fix.txt"), "the fix").unwrap();
1783        git(work.path(), &["add", "."]);
1784        git(work.path(), &["commit", "-m", "apply the fix"]);
1785        git(work.path(), &["push", "-u", "origin", "feat"]);
1786
1787        // Another clone rewrites the same change and force-pushes it
1788        let other = tempfile::tempdir().unwrap();
1789        init_clone(remote.path(), other.path());
1790        git(other.path(), &["checkout", "feat"]);
1791        git(
1792            other.path(),
1793            &["commit", "--amend", "-m", "apply the fix (rebased)"],
1794        );
1795        git(other.path(), &["push", "--force", "origin", "feat"]);
1796
1797        // orch now has an unpushed commit of its own on top of the stale branch
1798        std_fs::write(work.path().join("more.txt"), "follow-up").unwrap();
1799        git(work.path(), &["add", "."]);
1800        git(work.path(), &["commit", "-m", "follow-up"]);
1801
1802        let store = Arc::new(TaskStore::open_memory().await.unwrap());
1803        let store_id = store
1804            .create_internal("owner/repo", "Push recovery", "", "task", "3783", None)
1805            .await
1806            .unwrap();
1807        let task_id = format!("internal:{store_id}");
1808        let store_opt = Some(Arc::clone(&store));
1809        let ctx = StoreCtx {
1810            store: &store_opt,
1811            store_id_opt: Some(store_id),
1812            repo: "owner/repo",
1813            task_id: &task_id,
1814        };
1815
1816        let result =
1817            push_branch_with_log(&ctx, work.path(), "feat", "main", "claude", Some("opus")).await;
1818
1819        assert!(
1820            matches!(result, PushResult::Success),
1821            "orch must land the push itself after adopting the moved remote branch"
1822        );
1823
1824        let task = store.get(store_id).await.unwrap();
1825        assert!(
1826            task.last_error.is_empty(),
1827            "a recovered push must leave no error behind, got: {}",
1828            task.last_error
1829        );
1830        assert_eq!(task.push_failures, 0, "recovery is not a push failure");
1831
1832        // push_branch recovers the stale lease itself, so the log below is the proof: the
1833        // follow-up commit is on the remote, on top of the rebased history
1834        let log = Command::new("git")
1835            .current_dir(work.path())
1836            .args(["log", "--oneline", "origin/feat"])
1837            .output()
1838            .unwrap();
1839        let log = String::from_utf8_lossy(&log.stdout);
1840        assert!(
1841            log.contains("follow-up"),
1842            "remote must carry the task's commit: {log}"
1843        );
1844        assert!(
1845            log.contains("apply the fix (rebased)"),
1846            "orch must adopt the rebased remote history, not re-push its own: {log}"
1847        );
1848        assert!(
1849            !log.contains("apply the fix\n"),
1850            "the superseded commit must not come back: {log}"
1851        );
1852    }
1853
1854    #[test]
1855    fn missing_path_error_detects_io_not_found() {
1856        let err = anyhow::Error::from(io::Error::new(
1857            ErrorKind::NotFound,
1858            "simulated missing path",
1859        ));
1860        assert!(is_missing_path_error(&err));
1861    }
1862
1863    #[test]
1864    fn missing_path_error_detects_message_fallback() {
1865        let err = anyhow::anyhow!("push failed: No such file or directory (os error 2)");
1866        assert!(is_missing_path_error(&err));
1867    }
1868
1869    #[test]
1870    fn superseded_conflict_pr_requires_conflict_reroute_and_no_commits() {
1871        // Conflict reroute, nothing ahead of the default branch → close the PR.
1872        assert_eq!(superseded_conflict_pr(1, Some(42), false), Some(42));
1873        // Commits exist → the agent did work, keep the PR.
1874        assert_eq!(superseded_conflict_pr(1, Some(42), true), None);
1875        // No conflict reroute → an ordinary no-code run.
1876        assert_eq!(superseded_conflict_pr(0, Some(42), false), None);
1877        // No PR to close.
1878        assert_eq!(superseded_conflict_pr(1, None, false), None);
1879    }
1880
1881    /// End-to-end: a conflict reroute that comes back with zero commits ahead of
1882    /// the default branch closes the superseded PR and marks the task done,
1883    /// instead of blocking it as a same-agent no-code loop (#3780).
1884    #[tokio::test]
1885    async fn conflict_reroute_with_no_commits_closes_pr_and_marks_done() {
1886        fn git(dir: &Path, args: &[&str]) {
1887            let ok = Command::new("git")
1888                .args(args)
1889                .current_dir(dir)
1890                .env("GIT_AUTHOR_NAME", "t")
1891                .env("GIT_AUTHOR_EMAIL", "t@example.com")
1892                .env("GIT_COMMITTER_NAME", "t")
1893                .env("GIT_COMMITTER_EMAIL", "t@example.com")
1894                .output()
1895                .expect("git runs")
1896                .status
1897                .success();
1898            assert!(ok, "git {args:?} failed");
1899        }
1900
1901        // A worktree whose branch has nothing ahead of origin/main — the state
1902        // the agent leaves behind when the fix already landed on the base.
1903        let dir = tempfile::tempdir().unwrap();
1904        let origin = dir.path().join("origin.git");
1905        std_fs::create_dir(&origin).unwrap();
1906        git(&origin, &["init", "--bare", "-b", "main"]);
1907        let work_dir = dir.path().join("work");
1908        std_fs::create_dir(&work_dir).unwrap();
1909        git(&work_dir, &["init", "-b", "main"]);
1910        git(
1911            &work_dir,
1912            &["remote", "add", "origin", &origin.display().to_string()],
1913        );
1914        std_fs::write(work_dir.join("a.txt"), "a").unwrap();
1915        git(&work_dir, &["add", "-A"]);
1916        git(&work_dir, &["commit", "-m", "init"]);
1917        git(&work_dir, &["push", "-u", "origin", "main"]);
1918        git(&work_dir, &["checkout", "-b", "fix-branch"]);
1919
1920        let wt = worktree::WorktreeSetup {
1921            work_dir: work_dir.clone(),
1922            branch: "fix-branch".to_string(),
1923            default_branch: "main".to_string(),
1924            main_project_dir: work_dir.clone(),
1925        };
1926
1927        let store = Arc::new(TaskStore::open_memory().await.unwrap());
1928        let store_id = store
1929            .create(&NewTask {
1930                external_id: Some("3772".to_string()),
1931                repo: "owner/repo".to_string(),
1932                origin: "github".to_string(),
1933                title: "Fix the thing".to_string(),
1934                body: String::new(),
1935                source: "manual".to_string(),
1936                source_id: String::new(),
1937                author: "bot".to_string(),
1938                url: String::new(),
1939                labels: vec![],
1940                parent_id: None,
1941            })
1942            .await
1943            .unwrap();
1944        store
1945            .set_fields(store_id, &[("pr_number", serde_json::json!(42))])
1946            .await
1947            .unwrap();
1948        // review_poll rerouted this task to resolve a merge conflict.
1949        store
1950            .increment(store_id, "merge_conflict_retries")
1951            .await
1952            .unwrap();
1953        // A previous no-code run by the same agent — without the superseded
1954        // check this run would block the task as a same-agent loop.
1955        store
1956            .set_fields(
1957                store_id,
1958                &[("no_code_last_agent", serde_json::json!("claude"))],
1959            )
1960            .await
1961            .unwrap();
1962
1963        let parsed = agents::ParsedResponse {
1964            response: AgentResponse {
1965                status: "done".to_string(),
1966                summary: "the fix is already on main".to_string(),
1967                ..Default::default()
1968            },
1969            input_tokens: None,
1970            output_tokens: None,
1971            duration_ms: None,
1972        };
1973
1974        let (final_status, push_failed) = handle_success(
1975            "3772",
1976            parsed,
1977            &wt,
1978            "Fix the thing",
1979            "claude",
1980            Some("sonnet"),
1981            1,
1982            "owner/repo",
1983            &Some(Arc::clone(&store)),
1984            "",
1985        )
1986        .await
1987        .unwrap();
1988
1989        assert_eq!(
1990            final_status, "done",
1991            "a superseded conflict reroute must end done, not blocked"
1992        );
1993        assert!(!push_failed);
1994
1995        let task = store.get(store_id).await.unwrap();
1996        assert!(
1997            task.last_error.contains("superseded"),
1998            "the close attempt must be recorded on the task: {}",
1999            task.last_error
2000        );
2001        assert_eq!(
2002            task.pr_number, None,
2003            "a superseded PR must not stay on the task, the blocker refresh would read it as closed unmerged"
2004        );
2005    }
2006
2007    /// A conflict reroute with zero commits ahead marks the task done instead of
2008    /// blocking it as a same-agent no-code loop (#3780).
2009    #[test]
2010    fn classify_superseded_conflict_marks_done() {
2011        let status = classify_final_status(&DecisionInput {
2012            agent_status: "done",
2013            requires_pr: true,
2014            is_same_agent: true,
2015            is_superseded_conflict: true,
2016            ..Default::default()
2017        });
2018        assert_eq!(status, "done");
2019    }
2020
2021    // ── classify_final_status — one test per decision branch ─────────────────
2022
2023    /// Branch 1: workflow-scope push failure → block immediately (non-retryable).
2024    #[test]
2025    fn classify_workflow_scope_failure_blocks() {
2026        let status = classify_final_status(&DecisionInput {
2027            is_workflow_scope_failure: true,
2028            push_failed: true,
2029            push_failures: 1,
2030            ..Default::default()
2031        });
2032        assert_eq!(status, "blocked");
2033    }
2034
2035    /// Branch 1b: automatic rebase recovery failed (conflicts) → block immediately.
2036    #[test]
2037    fn classify_rebase_conflict_failure_blocks() {
2038        let status = classify_final_status(&DecisionInput {
2039            is_rebase_conflict_failure: true,
2040            push_failed: true,
2041            ..Default::default()
2042        });
2043        assert_eq!(status, "blocked");
2044    }
2045
2046    /// Branch 2a: push failed, < 3 times → reroute with "new".
2047    #[test]
2048    fn classify_push_failed_under_threshold_reroutes() {
2049        let status = classify_final_status(&DecisionInput {
2050            push_failed: true,
2051            push_failures: 2, // < 3
2052            ..Default::default()
2053        });
2054        assert_eq!(status, "new");
2055    }
2056
2057    #[test]
2058    fn denied_push_block_detected() {
2059        let resp = |s: &str| AgentResponse {
2060            status: "blocked".to_string(),
2061            summary: s.to_string(),
2062            ..Default::default()
2063        };
2064        assert!(is_denied_push_block(&resp(
2065            "Work committed, but `git push -u origin HEAD` was denied"
2066        )));
2067        assert!(is_denied_push_block(&resp(
2068            "It is not pushed: `git push --force-with-lease` needs approval that this session could not get."
2069        )));
2070        assert!(is_denied_push_block(&resp(
2071            "`git push --force-with-lease` was denied because this session cannot get approval"
2072        )));
2073        assert!(is_denied_push_block(&resp("The push requires approval")));
2074        assert!(!is_denied_push_block(&resp("Missing API key")));
2075    }
2076
2077    /// Branch 2b: push failed >= 3 times → block for human intervention.
2078    #[test]
2079    fn classify_push_failed_at_threshold_blocks() {
2080        let status = classify_final_status(&DecisionInput {
2081            push_failed: true,
2082            push_failures: 3, // >= 3
2083            ..Default::default()
2084        });
2085        assert_eq!(status, "blocked");
2086    }
2087
2088    /// Branch 3: done + PR exists → send to review.
2089    #[test]
2090    fn classify_done_with_pr_goes_to_needs_review() {
2091        let status = classify_final_status(&DecisionInput {
2092            agent_status: "done",
2093            has_pr: true,
2094            ..Default::default()
2095        });
2096        assert_eq!(status, "needs_review");
2097    }
2098
2099    /// Branch 4: done + delegations, no PR → block (waiting on child tasks).
2100    #[test]
2101    fn classify_done_with_delegations_no_pr_blocks() {
2102        let status = classify_final_status(&DecisionInput {
2103            agent_status: "done",
2104            has_delegations: true,
2105            ..Default::default()
2106        });
2107        assert_eq!(status, "blocked");
2108    }
2109
2110    /// Branch 5: done + commits pushed but PR creation failed → re-dispatch.
2111    #[test]
2112    fn classify_done_pushed_no_pr_reroutes_for_pr_creation() {
2113        let status = classify_final_status(&DecisionInput {
2114            agent_status: "done",
2115            has_pushed: true,
2116            ..Default::default()
2117        });
2118        assert_eq!(status, "routed");
2119    }
2120
2121    /// Branch 5b: done + pushed + PR base invalid → block (terminal error).
2122    #[test]
2123    fn classify_done_pushed_pr_base_invalid_blocks() {
2124        let status = classify_final_status(&DecisionInput {
2125            agent_status: "done",
2126            has_pushed: true,
2127            is_pr_base_invalid: true,
2128            ..Default::default()
2129        });
2130        assert_eq!(status, "blocked");
2131    }
2132
2133    /// Branch 5c: completed + pushed + PR base invalid → block (terminal error).
2134    #[test]
2135    fn classify_completed_pushed_pr_base_invalid_blocks() {
2136        let status = classify_final_status(&DecisionInput {
2137            agent_status: "completed",
2138            is_completed_status: true,
2139            has_pushed: true,
2140            is_pr_base_invalid: true,
2141            ..Default::default()
2142        });
2143        assert_eq!(status, "blocked");
2144    }
2145
2146    /// Branch 6a: done + requires_pr, under max reroutes → reroute.
2147    #[test]
2148    fn classify_done_requires_pr_under_max_reroutes() {
2149        let status = classify_final_status(&DecisionInput {
2150            agent_status: "done",
2151            requires_pr: true,
2152            no_code_reroutes: 1,
2153            max_reroutes: 3,
2154            is_same_agent: false,
2155            ..Default::default()
2156        });
2157        assert_eq!(status, "new");
2158    }
2159
2160    /// Branch 6a edge: same agent on no-code reroute → block immediately.
2161    #[test]
2162    fn classify_done_requires_pr_same_agent_blocks_immediately() {
2163        let status = classify_final_status(&DecisionInput {
2164            agent_status: "done",
2165            requires_pr: true,
2166            no_code_reroutes: 1,
2167            max_reroutes: 3,
2168            is_same_agent: true, // Same agent would be selected again
2169            ..Default::default()
2170        });
2171        assert_eq!(status, "blocked");
2172    }
2173
2174    /// Branch 6b: done + requires_pr, at max reroutes → block.
2175    #[test]
2176    fn classify_done_requires_pr_at_max_reroutes_blocks() {
2177        let status = classify_final_status(&DecisionInput {
2178            agent_status: "done",
2179            requires_pr: true,
2180            no_code_reroutes: 3,
2181            max_reroutes: 3,
2182            is_same_agent: false,
2183            ..Default::default()
2184        });
2185        assert_eq!(status, "blocked");
2186    }
2187
2188    /// Branch 6b edge: exceeding max also blocks.
2189    #[test]
2190    fn classify_done_requires_pr_exceeds_max_reroutes_blocks() {
2191        let status = classify_final_status(&DecisionInput {
2192            agent_status: "done",
2193            requires_pr: true,
2194            no_code_reroutes: 5,
2195            max_reroutes: 3,
2196            is_same_agent: false,
2197            ..Default::default()
2198        });
2199        assert_eq!(status, "blocked");
2200    }
2201
2202    /// Same agent on external task requiring PR → block immediately (not wait for max).
2203    #[test]
2204    fn classify_done_external_no_pushed_same_agent_blocks_immediately() {
2205        let status = classify_final_status(&DecisionInput {
2206            agent_status: "done",
2207            requires_pr: true, // always true for external tasks with !has_pr + done
2208            no_code_reroutes: 0,
2209            max_reroutes: 3,
2210            is_same_agent: true,
2211            ..Default::default()
2212        });
2213        assert_eq!(status, "blocked");
2214    }
2215
2216    /// External task with no pushed commits: reroute (agent must produce commits
2217    /// before the issue can be closed). This was the source of issue #1898 — an
2218    /// agent claiming "already implemented" without any code changes would
2219    /// previously match non-code keywords and close the issue falsely.
2220    ///
2221    /// External tasks with done+!has_pr+!has_pushed always have requires_pr=true
2222    /// (see handle_success), so they are caught by branch 6, not branch 7.
2223    #[test]
2224    fn classify_done_external_no_pushed_commits_reroutes() {
2225        let status = classify_final_status(&DecisionInput {
2226            agent_status: "done",
2227            has_pushed: false,
2228            requires_pr: true, // always true for external tasks with !has_pr + done
2229            no_code_reroutes: 0,
2230            max_reroutes: 3,
2231            is_same_agent: false,
2232            ..Default::default()
2233        });
2234        assert_eq!(status, "new");
2235    }
2236
2237    /// Internal task with no pushed commits may still be marked done — internal
2238    /// tasks may produce no git-visible changes.
2239    #[test]
2240    fn classify_done_internal_task_no_pr_is_done() {
2241        let status = classify_final_status(&DecisionInput {
2242            agent_status: "done",
2243            has_pushed: false,
2244            requires_pr: false,
2245            ..Default::default()
2246        });
2247        assert_eq!(status, "done");
2248    }
2249
2250    /// Pass-through: non-"done" status is returned unchanged.
2251    #[test]
2252    fn classify_non_done_status_passes_through() {
2253        for status_str in &["in_progress", "needs_review", "blocked"] {
2254            let result = classify_final_status(&DecisionInput {
2255                agent_status: status_str,
2256                ..Default::default()
2257            });
2258            assert_eq!(
2259                result, *status_str,
2260                "status '{status_str}' should pass through unchanged"
2261            );
2262        }
2263    }
2264
2265    /// Descriptive completion text should normalize to done.
2266    #[test]
2267    fn classify_descriptive_completion_status_normalizes_to_done() {
2268        let status = classify_final_status(&DecisionInput {
2269            agent_status: "Trading scan complete. Nothing to trade.",
2270            ..Default::default()
2271        });
2272        assert_eq!(status, "done");
2273    }
2274
2275    /// Do not normalize to done when completion-like text includes failure cues.
2276    #[test]
2277    fn classify_descriptive_completion_with_failure_cues_does_not_normalize() {
2278        let status = classify_final_status(&DecisionInput {
2279            agent_status: "completed but failed to push branch due to error",
2280            ..Default::default()
2281        });
2282        assert_eq!(status, "completed but failed to push branch due to error");
2283    }
2284
2285    /// push_failed takes precedence over done+has_pr (push failure detected first).
2286    #[test]
2287    fn classify_push_failed_takes_precedence_over_has_pr() {
2288        let status = classify_final_status(&DecisionInput {
2289            agent_status: "done",
2290            has_pr: true,
2291            push_failed: true,
2292            push_failures: 1,
2293            ..Default::default()
2294        });
2295        // push_failed branch runs before done+has_pr check
2296        assert_eq!(status, "new");
2297    }
2298
2299    /// workflow_scope_failure takes precedence over push_failed < 3.
2300    #[test]
2301    fn classify_workflow_scope_failure_takes_precedence_over_push_reroute() {
2302        let status = classify_final_status(&DecisionInput {
2303            is_workflow_scope_failure: true,
2304            push_failed: true,
2305            push_failures: 1, // would normally reroute
2306            ..Default::default()
2307        });
2308        assert_eq!(status, "blocked");
2309    }
2310
2311    // ── "completed" status tests ────────────────────────────────────────────────
2312
2313    /// "completed" + has_pr → needs_review.
2314    #[test]
2315    fn classify_completed_with_pr_goes_to_needs_review() {
2316        let status = classify_final_status(&DecisionInput {
2317            agent_status: "completed",
2318            has_pr: true,
2319            is_completed_status: true,
2320            ..Default::default()
2321        });
2322        assert_eq!(status, "needs_review");
2323    }
2324
2325    /// "completed" + delegations, no PR → blocked.
2326    #[test]
2327    fn classify_completed_with_delegations_no_pr_blocks() {
2328        let status = classify_final_status(&DecisionInput {
2329            agent_status: "completed",
2330            has_delegations: true,
2331            is_completed_status: true,
2332            ..Default::default()
2333        });
2334        assert_eq!(status, "blocked");
2335    }
2336
2337    /// "completed" + pushed, no PR → routed for PR creation.
2338    #[test]
2339    fn classify_completed_pushed_no_pr_reroutes_for_pr_creation() {
2340        let status = classify_final_status(&DecisionInput {
2341            agent_status: "completed",
2342            has_pushed: true,
2343            is_completed_status: true,
2344            ..Default::default()
2345        });
2346        assert_eq!(status, "routed");
2347    }
2348
2349    /// "completed" + external task requiring PR, under max reroutes → reroute.
2350    #[test]
2351    fn classify_completed_requires_pr_under_max_reroutes() {
2352        let status = classify_final_status(&DecisionInput {
2353            agent_status: "completed",
2354            requires_pr: true,
2355            no_code_reroutes: 1,
2356            max_reroutes: 3,
2357            is_completed_status: true,
2358            is_same_agent: false,
2359            ..Default::default()
2360        });
2361        assert_eq!(status, "new");
2362    }
2363
2364    /// "completed" + external task, at max reroutes → block.
2365    #[test]
2366    fn classify_completed_requires_pr_at_max_reroutes_blocks() {
2367        let status = classify_final_status(&DecisionInput {
2368            agent_status: "completed",
2369            requires_pr: true,
2370            no_code_reroutes: 3,
2371            max_reroutes: 3,
2372            is_completed_status: true,
2373            is_same_agent: false,
2374            ..Default::default()
2375        });
2376        assert_eq!(status, "blocked");
2377    }
2378
2379    /// "completed" + external task, same agent on no-code reroute → block immediately.
2380    #[test]
2381    fn classify_completed_same_agent_blocks_immediately() {
2382        let status = classify_final_status(&DecisionInput {
2383            agent_status: "completed",
2384            requires_pr: true,
2385            no_code_reroutes: 1,
2386            max_reroutes: 3,
2387            is_completed_status: true,
2388            is_same_agent: true,
2389            ..Default::default()
2390        });
2391        assert_eq!(status, "blocked");
2392    }
2393
2394    /// "completed" on internal task (no requires_pr) → mark done.
2395    #[test]
2396    fn classify_completed_internal_task_is_done() {
2397        let status = classify_final_status(&DecisionInput {
2398            agent_status: "completed",
2399            requires_pr: false,
2400            is_completed_status: true,
2401            ..Default::default()
2402        });
2403        assert_eq!(status, "done");
2404    }
2405
2406    // ── agent_blocked_reason — regression tests for issue #3012 ──────────────
2407
2408    /// Agent explicitly returned blocked with an error field — use error as reason.
2409    #[test]
2410    fn agent_blocked_reason_prefers_error_field() {
2411        let resp = AgentResponse {
2412            status: "blocked".to_string(),
2413            error: Some("CDP endpoint not reachable".to_string()),
2414            summary: "Could not complete research".to_string(),
2415            remaining: vec!["Run x-twitter-brave with reachable endpoint".to_string()],
2416            ..Default::default()
2417        };
2418        let reason = agent_blocked_reason(&resp);
2419        assert!(
2420            reason.contains("CDP endpoint not reachable"),
2421            "reason: {reason}"
2422        );
2423        assert!(reason.contains("Remaining:"), "reason: {reason}");
2424        assert!(reason.contains("x-twitter-brave"), "reason: {reason}");
2425    }
2426
2427    /// No error field — falls back to summary.
2428    #[test]
2429    fn agent_blocked_reason_falls_back_to_summary() {
2430        let resp = AgentResponse {
2431            status: "blocked".to_string(),
2432            error: None,
2433            summary: "Could not access required data sources in sandboxed environment".to_string(),
2434            remaining: vec![],
2435            ..Default::default()
2436        };
2437        let reason = agent_blocked_reason(&resp);
2438        assert_eq!(
2439            reason,
2440            "Could not access required data sources in sandboxed environment"
2441        );
2442    }
2443
2444    /// No error or summary — returns generic fallback.
2445    #[test]
2446    fn agent_blocked_reason_generic_fallback() {
2447        let resp = AgentResponse {
2448            status: "blocked".to_string(),
2449            ..Default::default()
2450        };
2451        let reason = agent_blocked_reason(&resp);
2452        assert_eq!(reason, "agent returned blocked status without a reason");
2453    }
2454
2455    /// Agent-returned "blocked" status passes through classify_final_status unchanged.
2456    #[test]
2457    fn classify_agent_blocked_passes_through() {
2458        let status = classify_final_status(&DecisionInput {
2459            agent_status: "blocked",
2460            ..Default::default()
2461        });
2462        assert_eq!(status, "blocked");
2463    }
2464
2465    #[test]
2466    fn classify_retryable_agent_blocked_reroutes() {
2467        let status = classify_final_status(&DecisionInput {
2468            agent_status: "blocked",
2469            is_retryable_blocked: true,
2470            ..Default::default()
2471        });
2472        assert_eq!(status, "new");
2473    }
2474
2475    #[test]
2476    fn retryable_blocked_reason_detects_transient_patterns() {
2477        assert!(is_retryable_blocked_reason(
2478            "Twitter access unavailable: service timed out"
2479        ));
2480        assert!(is_retryable_blocked_reason(
2481            "final commit blocked by git worktree lock permission error"
2482        ));
2483        assert!(is_retryable_blocked_reason(
2484            "could not commit due a worktree git lock permission error"
2485        ));
2486        assert!(is_retryable_blocked_reason(
2487            "git lockfile permission restrictions in this environment"
2488        ));
2489        assert!(is_retryable_blocked_reason(
2490            "API rate limit hit, try again later"
2491        ));
2492        assert!(is_retryable_blocked_reason(
2493            "Credential bean/hyperliquid-address exists in the pass store but GPG decryption fails in this agent session — no passphrase available."
2494        ));
2495        assert!(is_retryable_blocked_reason(
2496            "credential resolution failed via PasswordStoreProvider; AgentVaultProvider unavailable"
2497        ));
2498    }
2499
2500    #[test]
2501    fn retryable_blocked_reason_rejects_permanent_patterns() {
2502        assert!(!is_retryable_blocked_reason(
2503            "invalid request payload: malformed JSON"
2504        ));
2505        assert!(!is_retryable_blocked_reason(
2506            "resource not found in repository"
2507        ));
2508    }
2509}