Skip to main content

orch/engine/runner/
response_handler.rs

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