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