Add retry and code review step.

Author: beacoder

README.md

@@ -7,6 +7,11 @@ A Python port of the Emacs [gptel-agent-harness](https://github.com/beacoder/gpt
 - **Agent loop with completion supervision** — the model is nudged (max 2) when it
   tries to stop before the task is complete; the nudge counter resets on tool
   calls; tool results are sanitized so a failed call never strands the loop.
+- **API retry with backoff** — transient failures (HTTP 429 / 5xx, connection
+  errors) are retried automatically with exponential backoff + jitter
+  (honoring `Retry-After`), so a rate limit or a dropped connection no longer
+  kills the run; retries never duplicate streamed output and a Ctrl-C aborts
+  the backoff wait promptly. Permanent errors (other 4xx) fail fast.
 - **Context management** — CJK-aware token estimation, per-model context
   windows (deepseek-v4/glm-5.2 1M, gpt-5 400k, kimi-k2.7 256k, claude 200k,
   ...), self-calibrating estimates from API-reported input tokens, and

python_agent_harness/agent.py

@@ -421,6 +421,9 @@ def safe_delta(text: str) -> None:
                     # sub-agents must not stream into the parent's live
                     # stream row — their text is private until returned
                     on_delta=(safe_delta if self.top_level else None),
+                    # poll cancellation during retry backoff so Ctrl-C
+                    # aborts promptly instead of after the full sleep
+                    cancel_check=self._is_cancelled,
                 )
             except Exception as e:  # noqa: BLE001 - API errors become ERRS
                 if self._is_cancelled():

python_agent_harness/client.py

@@ -9,6 +9,7 @@
 
 import json
 import os
+import random
 import time
 from pathlib import Path
 from typing import Any, Callable, Iterator
@@ -23,6 +24,46 @@ class ApiError(Exception):
     """Raised when the API call itself fails (network/HTTP)."""
 
 
+class RetryableApiError(ApiError):
+    """A transient failure (rate limit, server error) safe to retry.
+
+    Carries the server's ``Retry-After`` value (if any) so the retry
+    backoff can honor it.  Permanent errors remain a plain ApiError.
+    """
+
+    def __init__(self, message: str, retry_after: str | None = None) -> None:
+        super().__init__(message)
+        self.retry_after = retry_after
+
+
+def _retryable_status(status: int) -> bool:
+    """429 and 5xx are transient; every other error is permanent."""
+    return status == 429 or status >= 500
+
+
+def _retry_delay(
+    attempt: int,
+    retry_after: str | None,
+    base_delay: float,
+    max_delay: float,
+) -> float:
+    """Backoff delay for the failed ATTEMPT (1 = first attempt).
+
+    Computed as ``base_delay`` doubled per attempt, capped at
+    ``max_delay``, plus jitter.  A ``Retry-After`` header (seconds)
+    from a 429 response wins when present.
+    """
+    if isinstance(retry_after, str) and retry_after.strip():
+        try:
+            secs = float(retry_after.strip())
+        except ValueError:
+            pass
+        else:
+            return min(secs, max_delay) + random.uniform(0, 0.5)
+    delay = min(base_delay * (2 ** (attempt - 1)), max_delay)
+    return delay + random.uniform(0, delay * 0.3)
+
+
 def _llm_log_path() -> Path:
     """Return the LLM log file path for a new session."""
     import uuid
@@ -113,12 +154,22 @@ def __init__(
         model: str | None = None,
         timeout: float = 600.0,
         verify: str | bool | None = None,
+        retry_max: int | None = None,
+        retry_base_delay: float | None = None,
+        retry_max_delay: float | None = None,
     ) -> None:
         self.base_url = (base_url or config.DEFAULT_BASE_URL).rstrip("/")
         self.api_key = api_key or _default_api_key()
         self.model = model or config.DEFAULT_MODEL
         self.timeout = timeout
         self.verify = verify if verify is not None else _resolve_ca_bundle()
+        self.retry_max = config.API_RETRY_MAX if retry_max is None else retry_max
+        self.retry_base_delay = (
+            config.API_RETRY_BASE_DELAY if retry_base_delay is None else retry_base_delay
+        )
+        self.retry_max_delay = (
+            config.API_RETRY_MAX_DELAY if retry_max_delay is None else retry_max_delay
+        )
         self._http = httpx.Client(timeout=timeout, verify=self.verify)
         self.log_path: Path | None = _llm_log_path() if config.LLM_LOG_ENABLED else None
 
@@ -201,6 +252,7 @@ def chat(
         on_delta: Callable[[str], None] | None = None,
         on_tool_call: Callable[[str, str, str], None] | None = None,
         stream: bool = True,
+        cancel_check: Callable[[], bool] | None = None,
     ) -> tuple[Message, Usage]:
         """Send a chat request, return (assistant msg, usage).
 
@@ -210,24 +262,62 @@ def chat(
         ``stream`` False a single non-streaming POST is used; both
         callbacks fire once per text/tool-call with the complete values,
         so callers (agent loop, TUI) behave identically either way.
+
+        Transient failures (HTTP 429 / 5xx, connection errors) are
+        retried with exponential backoff + jitter up to ``retry_max``
+        attempts, honoring ``Retry-After`` when present.  A retry only
+        happens before any delta has been delivered to the callbacks,
+        so streaming output is never duplicated for the caller.  Other
+        4xx errors are permanent and fail immediately.  ``cancel_check``
+        (when given) is polled during backoff sleeps so an abort lands
+        promptly instead of after the full wait.
         """
         payload = self._payload(
             messages, tools, stream=stream, temperature=temperature,
             max_tokens=max_tokens, system=system,
             reasoning_effort=reasoning_effort,
         )
         usage = Usage()
-        try:
-            if stream:
-                content_parts, reasoning_parts, tc_index = self._stream_response(
-                    payload, on_delta, on_tool_call, usage
-                )
-            else:
-                content_parts, reasoning_parts, tc_index = self._sync_response(
-                    payload, on_delta, on_tool_call, usage
-                )
-        except httpx.HTTPError as e:
-            raise ApiError(f"network error: {e}") from e
+        emitted = False
+
+        def wrap_delta(chunk: str) -> None:
+            nonlocal emitted
+            emitted = True
+            if on_delta:
+                on_delta(chunk)
+
+        def wrap_tool_call(name: str, call_id: str, fragment: str) -> None:
+            nonlocal emitted
+            emitted = True
+            if on_tool_call:
+                on_tool_call(name, call_id, fragment)
+
+        attempt = 0
+        while True:
+            attempt += 1
+            try:
+                if stream:
+                    content_parts, reasoning_parts, tc_index = self._stream_response(
+                        payload, wrap_delta, wrap_tool_call, usage
+                    )
+                else:
+                    content_parts, reasoning_parts, tc_index = self._sync_response(
+                        payload, wrap_delta, wrap_tool_call, usage
+                    )
+                break
+            except RetryableApiError as e:
+                if emitted or attempt >= self.retry_max:
+                    raise
+                if self._sleep_backoff(attempt, e.retry_after, cancel_check):
+                    raise
+            except httpx.HTTPError as e:
+                # connection-level failures: connect errors, timeouts,
+                # dropped streams — all transient unless a delta already
+                # reached the caller (then a retry would duplicate it)
+                if emitted or attempt >= self.retry_max:
+                    raise ApiError(f"network error: {e}") from e
+                if self._sleep_backoff(attempt, None, cancel_check):
+                    raise ApiError(f"network error: {e}") from e
 
         content = "".join(content_parts)
         tool_calls = None
@@ -249,6 +339,31 @@ def chat(
         _log_llm_interaction(self.log_path, payload, msg, usage)
         return msg, usage
 
+    def _sleep_backoff(
+        self,
+        attempt: int,
+        retry_after: str | None,
+        cancel_check: Callable[[], bool] | None,
+    ) -> bool:
+        """Sleep between retries; return True when aborted (cancelled).
+
+        ``attempt`` is the number of the request that just failed (1 =
+        first attempt); the delay doubles per attempt, capped, with
+        jitter (``Retry-After`` wins for 429s).  When ``cancel_check``
+        is given it is polled in small increments so a Ctrl-C lands
+        promptly instead of after the full backoff wait.
+        """
+        deadline = time.monotonic() + _retry_delay(
+            attempt, retry_after, self.retry_base_delay, self.retry_max_delay
+        )
+        while True:
+            remaining = deadline - time.monotonic()
+            if remaining <= 0:
+                return False
+            if cancel_check is not None and cancel_check():
+                return True
+            time.sleep(min(0.25, remaining))
+
     def _stream_response(
         self,
         payload: dict[str, Any],
@@ -271,9 +386,12 @@ def _stream_response(
         ) as resp:
             if resp.status_code >= 400:
                 body = resp.read().decode("utf-8", "replace")
-                raise ApiError(
-                    f"API error {resp.status_code}: {body[:500]}"
-                )
+                message = f"API error {resp.status_code}: {body[:500]}"
+                if _retryable_status(resp.status_code):
+                    raise RetryableApiError(
+                        message, resp.headers.get("Retry-After")
+                    )
+                raise ApiError(message)
             for chunk in _iter_sse(resp.iter_lines()):
                 if not chunk:
                     continue
@@ -324,9 +442,12 @@ def _sync_response(
             self._url(), headers=self._headers(stream=False), json=payload
         )
         if resp.status_code >= 400:
-            raise ApiError(
-                f"API error {resp.status_code}: {resp.text[:500]}"
-            )
+            message = f"API error {resp.status_code}: {resp.text[:500]}"
+            if _retryable_status(resp.status_code):
+                raise RetryableApiError(
+                    message, resp.headers.get("Retry-After")
+                )
+            raise ApiError(message)
         data = resp.json()
         u = data.get("usage")
         if u:

python_agent_harness/config.py

@@ -147,6 +147,15 @@
 MAX_TOKENS = 8192
 TEMPERATURE = 0.0
 
+# ---- API retry / backoff -------------------------------------------------------
+# Transient API failures (HTTP 429 / 5xx, connection errors) are retried
+# with exponential backoff + jitter instead of killing the run.  The
+# per-request attempt budget and delay bounds live here; a Client
+# instance can override them per call.
+API_RETRY_MAX = 3            # max attempts per request (initial + retries)
+API_RETRY_BASE_DELAY = 1.0   # base backoff (seconds), doubled per attempt
+API_RETRY_MAX_DELAY = 30.0   # per-attempt backoff cap (seconds)
+
 # ---- tool execution ----------------------------------------------------------
 SUBAGENT_MAX_ROUNDS = 60
 # Max tool calls that may run CONCURRENTLY in one tool round (all tools

python_agent_harness/prompts/agent.txt

@@ -67,7 +67,7 @@ The user will primarily request you perform software engineering tasks. This inc
 - Use the available search tools to understand the codebase and the user's query. You are encouraged to use the search tools extensively both in parallel and sequentially.
 - Implement the solution using all tools available to you
 - Verify the solution if possible with tests. NEVER assume specific test framework or test script. Check the README or search codebase to determine the testing approach.
-- VERY IMPORTANT: When you have completed a task, you MUST run the lint and typecheck commands (e.g. npm run lint, npm run typecheck, ruff, etc.) with Bash if they were provided to you to ensure your code is correct. If you are unable to find the correct command, ask the user for the command to run and if they supply it, proactively suggest writing it to AGENTS.md so that you will know to run it next time. You MUST also review the updated code carefully for any hidden issues before considering the task finished.
+- VERY IMPORTANT: When you have completed a task, you MUST run the lint and typecheck commands (e.g. npm run lint, npm run typecheck, ruff, etc.) with Bash if they were provided to you to ensure your code is correct. If you are unable to find the correct command, ask the user for the command to run and if they supply it, proactively suggest writing it to AGENTS.md so that you will know to run it next time.
 NEVER commit changes unless the user explicitly asks you to. It is VERY IMPORTANT to only commit when explicitly asked, otherwise the user will feel that you are being too proactive.
 
 - Tool results and user messages may include <system-reminder> tags. <system-reminder> tags contain useful information and reminders. They are NOT part of the user's provided input or the tool result.

python_agent_harness/prompts/task-completion-rules.txt

@@ -12,6 +12,7 @@ Completion criteria:
 Before finishing, you MUST:
 
 - Explicitly check whether the task goal is achieved.
+- Carefully review the updated code for any hidden issues.
 - If there is any uncertainty, assume the task is NOT complete.
 - If a tool execution failed, you MUST retry or choose an alternative approach.
 

tests/fake_openai_server.py

@@ -9,6 +9,12 @@
 # Queue of non-streaming response bodies consumed in order; when
 # exhausted, NON_STREAM_RESPONSE / the default reply is used.
 NON_STREAM_SEQUENCE: list[dict] = []
+# Queue of HTTP statuses consumed in order; a non-200 entry makes the
+# server respond with that error status instead of a reply (used to
+# exercise client retry behavior).  When exhausted, 200 is used.
+STATUS_QUEUE: list[int] = []
+# Optional Retry-After header (seconds) attached to error responses.
+RETRY_AFTER_HEADER: str | None = None
 # Every request body received, in order (for asserting payloads).
 REQUEST_BODIES: list[dict] = []
 
@@ -17,14 +23,27 @@ def reset_state() -> None:
     global NON_STREAM_RESPONSE
     NON_STREAM_RESPONSE = None
     NON_STREAM_SEQUENCE.clear()
+    STATUS_QUEUE.clear()
     REQUEST_BODIES.clear()
+    global RETRY_AFTER_HEADER
+    RETRY_AFTER_HEADER = None
 
 
 class Handler(BaseHTTPRequestHandler):
     def do_POST(self):
         length = int(self.headers.get("Content-Length", 0))
         body = json.loads(self.rfile.read(length) or b"{}")
         REQUEST_BODIES.append(body)
+        status = STATUS_QUEUE.pop(0) if STATUS_QUEUE else 200
+        if status != 200:
+            err = b'{"error": "transient failure"}'
+            self.send_response(status)
+            if RETRY_AFTER_HEADER is not None:
+                self.send_header("Retry-After", RETRY_AFTER_HEADER)
+            self.send_header("Content-Length", str(len(err)))
+            self.end_headers()
+            self.wfile.write(err)
+            return
         stream = body.get("stream", False)
         if stream:
             chunks = [

tests/test_agent.py

@@ -28,12 +28,13 @@ def __init__(self, script):
         self.kwargs = []
 
     def chat(self, messages, tools=None, system=None, temperature=None,
-             max_tokens=None, reasoning_effort=None, on_delta=None, stream=True):
+             max_tokens=None, reasoning_effort=None, on_delta=None, stream=True,
+             cancel_check=None):
         self.calls.append([m.to_api() for m in messages])
         self.kwargs.append({
             "tools": tools, "system": system, "temperature": temperature,
             "max_tokens": max_tokens, "reasoning_effort": reasoning_effort,
-            "stream": stream,
+            "stream": stream, "cancel_check": cancel_check,
         })
         if not self.script:
             return Message(role="assistant", content="done"), Usage()