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