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