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