Skip to main content

orch/engine/runner/
response_handler.rs

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