diff --git a/tests/tools/test_delegate.py b/tests/tools/test_delegate.py index a5aa206a65..ec09b6e374 100644 --- a/tests/tools/test_delegate.py +++ b/tests/tools/test_delegate.py @@ -770,6 +770,47 @@ class TestDelegateFailedChildStatus(unittest.TestCase): self.assertEqual(entry["exit_reason"], "error") self.assertFalse(entry["truncated"]) + def test_error_without_failed_flag_marks_failed(self): + """A child result that carries a non-empty error string but OMITS the + ``failed`` key entirely (not ``failed=False`` — the key is absent, as in + legacy/partial result dicts) must still be status=failed + exit_reason=error. + The status branch checks ``result.get('failed') or result.get('error')``, + so the error field alone has to win — otherwise a dropped ``failed`` key + would silently mislabel a provider rejection as budget exhaustion.""" + entry = self._delegate_single( + { + "final_response": "connection reset while streaming", + "completed": False, + "interrupted": False, + "error": "connection reset", + "api_calls": 2, + "messages": [], + } + ) + self.assertEqual(entry["status"], "failed") + self.assertEqual(entry["exit_reason"], "error") + self.assertFalse(entry["truncated"]) + + def test_empty_error_with_summary_is_completed(self): + """REGRESSION PIN: an empty-string ``error`` field must NOT be treated as + a failure. ``result.get('error')`` returns ``''`` which is falsy, so the + failure branch correctly falls through to the summary-presence heuristic. + Empty error + a real summary => status=completed, exit_reason=completed + (or max_iterations if completed=False), never 'error'.""" + entry = self._delegate_single( + { + "final_response": "work produced", + "completed": True, + "interrupted": False, + "error": "", + "api_calls": 2, + "messages": [], + } + ) + self.assertEqual(entry["status"], "completed") + self.assertEqual(entry["exit_reason"], "completed") + self.assertFalse(entry["truncated"]) + def test_genuine_truncation_stays_completed_max_iterations(self): """REGRESSION GUARD: a child that genuinely exhausts its iteration budget (completed=False, no failed flag, no error) but still returns a diff --git a/tools/delegate_tool.py b/tools/delegate_tool.py index 1d5f7f8ec2..89cab103c5 100644 --- a/tools/delegate_tool.py +++ b/tools/delegate_tool.py @@ -2579,7 +2579,27 @@ def _run_single_child( ) -> Dict[str, Any]: """ Run a pre-built child agent. Called from within a thread. - Returns a structured result dict. + Returns a structured result dict with a ``status`` and ``exit_reason`` + that are derived honestly from the child's structured completion fields. + + ``status`` ∈ {``"completed"``, ``"interrupted"``, ``"failed"``}: + * ``"completed"`` — the child reached a normal finish (may still have + hit its iteration budget; see ``exit_reason``). + * ``"interrupted"`` — the child was interrupted (``interrupted=True``). + * ``"failed"`` — a structured failure (``failed=True`` or a non-empty + ``error``) or a summary-less/invalid terminal state. + + ``exit_reason`` ∈ {``"completed"``, ``"max_iterations"``, ``"interrupted"``, + ``"error"``}: + * ``"completed"`` — normal finish. + * ``"max_iterations"`` — genuine per-child iteration-budget exhaustion + (``completed=False`` with no failure fields). + * ``"interrupted"`` — interrupted by the parent. + * ``"error"`` — provider rejection / terminal failure; NOT + budget exhaustion (this is the case #97655 fixed). + + ``truncated`` is derived as ``exit_reason == "max_iterations"`` only, so the + parent-visible truncation flag stays truthful for all of the above. """ child_start = time.monotonic() @@ -3238,6 +3258,10 @@ def _run_single_child( _output_tokens = getattr(child, "session_completion_tokens", 0) _model = getattr(child, "model", None) + # --- result entry contract (see _run_single_child docstring) --- + # status ∈ {completed, interrupted, failed} + # exit_reason ∈ {completed, max_iterations, interrupted, error} + # truncated is exactly (exit_reason == "max_iterations"). entry: Dict[str, Any] = { "task_index": task_index, "status": status,