fix: improve operational error reporting across providers

.env.example

@@ -50,6 +50,7 @@ OPENAI_API_KEY=your_openai_api_key_here
 LLM_MAX_RETRIES=3
 LLM_RETRY_BASE_DELAY_S=0.5
 LLM_RETRY_MAX_DELAY_S=10
+RETRY_AFTER_FALLBACK_S=60
 
 # External Tools & Sandboxing
 # Optional: web_search is omitted from the tool registry when this is unset.

README.md

@@ -127,6 +127,7 @@ API_KEYS=dev_key_1,dev_key_2
 LLM_MAX_RETRIES=3
 LLM_RETRY_BASE_DELAY_S=0.5
 LLM_RETRY_MAX_DELAY_S=10
+RETRY_AFTER_FALLBACK_S=60
 ```
 
 ### 4. Running with Docker Compose (Recommended)

agentkit/config.py

@@ -238,6 +238,11 @@ class Settings(BaseSettings):
         gt=0,
         description="Maximum delay in seconds between LLM retry attempts.",
     )
+    RETRY_AFTER_FALLBACK_S: int = Field(
+        default=60,
+        gt=0,
+        description="Retry-After fallback in seconds when a provider omits it.",
+    )
     FILE_TOOL_BASE_DIR: str = Field(
         default="/tmp/agentkit_sandbox",
         description="Base directory for sandboxed file reading operations.",

agentkit/core/agent.py

@@ -405,7 +405,7 @@ async def _finalize_canceled(*, save_history: bool) -> None:
                                             f"Detected duplicate tool call loop: '{call.name}' called "
                                             f"{repeat_count} times with identical arguments within "
                                             f"the last {self.config.duplicate_tool_call_window} "
-                                            f"tool calls: {call.arguments}"
+                                            "tool calls with identical arguments."
                                         ),
                                         steps_count=steps_count,
                                         total_input_tokens=total_input_tokens,
@@ -417,9 +417,7 @@ async def _finalize_canceled(*, save_history: bool) -> None:
                                         raise DuplicateToolCallLoopError(call.name, repeat_count)
                                     return res
 
-                                logger.info(
-                                    "Executing tool '%s' with args: %s", call.name, call.arguments
-                                )
+                                logger.info("Executing tool '%s'", call.name)
                                 result = await self.registry.execute(
                                     call,
                                     timeout_s=self.config.tool_timeout_s,
@@ -442,7 +440,7 @@ async def _finalize_canceled(*, save_history: bool) -> None:
                                 )
 
                                 if self.trace is not None and hasattr(self.trace, "record"):
-                                    with contextlib.suppress(Exception):
+                                    try:
                                         trace_record = StepTrace(
                                             run_id=run_id,
                                             step_no=steps_count,
@@ -464,6 +462,13 @@ async def _finalize_canceled(*, save_history: bool) -> None:
                                         )
                                         await self.trace.record(trace_record)
                                         traced_turn_usage.add(steps_count)
+                                    except Exception as exc:
+                                        logger.warning(
+                                            "Could not record tool trace (run_id=%s, step_no=%s): %s",
+                                            run_id,
+                                            steps_count,
+                                            exc,
+                                        )
                         else:
                             # No tool calls: this is the final answer, a provider refusal, or
                             # an empty completion. Only a non-empty answer is a success; the

agentkit/llm/factory.py

@@ -74,6 +74,11 @@ def create_llm_client_from_settings(settings: Settings) -> LLMClient:
         provider=provider,
         api_key=api_key,
         model=settings.LLM_MODEL,
+        **(
+            {"retry_after_fallback_s": settings.RETRY_AFTER_FALLBACK_S}
+            if provider == "openai"
+            else {}
+        ),
     )
 
     if provider in ("gemini", "openai") and settings.LLM_MAX_RETRIES > 0:

agentkit/llm/gemini.py

@@ -36,6 +36,21 @@ def _enum_label(value: Any) -> str | None:
     return candidate if _ENUM_LABEL_REGEX.match(candidate) else None
 
 
+def _has_invalid_api_key_reason(details: Any) -> bool:
+    """Inspect structured Google error details for the API-key rejection reason."""
+    if isinstance(details, dict):
+        if details.get("reason") == "API_KEY_INVALID":
+            return True
+        return any(
+            _has_invalid_api_key_reason(value)
+            for key, value in details.items()
+            if key in {"error", "details", "errors"}
+        )
+    if isinstance(details, list):
+        return any(_has_invalid_api_key_reason(item) for item in details)
+    return False
+
+
 class GeminiClient(LLMClient):
     """LLM client adapter for Google Gemini models via google-genai."""
 
@@ -186,9 +201,17 @@ async def chat(
             )
         except Exception as exc:
             err_str = str(exc)
-            if "API_KEY_INVALID" in err_str or "unauthenticated" in err_str.lower():
+            status_code = getattr(exc, "code", None)
+            if status_code is None:
+                status_code = getattr(exc, "status_code", None)
+            provider_status = getattr(exc, "status", None)
+            details = getattr(exc, "details", None)
+            if (
+                provider_status in {"UNAUTHENTICATED", "PERMISSION_DENIED"}
+                or _has_invalid_api_key_reason(details)
+                or status_code == 401
+            ):
                 raise AuthenticationError(f"Gemini API key is invalid: {err_str}") from exc
-            status_code = getattr(exc, "code", getattr(exc, "status_code", None))
             raise LLMProviderError(
                 provider="gemini",
                 message=err_str,

agentkit/llm/openai.py

@@ -3,8 +3,10 @@
 import inspect
 import json
 import logging
-import os
+import math
 import uuid
+from datetime import UTC, datetime
+from email.utils import parsedate_to_datetime
 from typing import Any
 
 import openai
@@ -17,6 +19,42 @@
 logger = logging.getLogger(__name__)
 
 
+def _retry_after_seconds(exc: openai.RateLimitError, fallback_s: int) -> int:
+    """Return the provider's retry delay, or the configured fallback if absent/invalid."""
+    response = getattr(exc, "response", None)
+    headers = getattr(response, "headers", None)
+    if headers is None:
+        return fallback_s
+
+    retry_after_ms = headers.get("retry-after-ms")
+    if retry_after_ms is not None:
+        try:
+            delay_s = float(retry_after_ms) / 1000
+        except (TypeError, ValueError):
+            delay_s = math.nan
+        if math.isfinite(delay_s) and delay_s >= 0:
+            return math.ceil(delay_s)
+
+    retry_after = headers.get("retry-after")
+    if retry_after is None:
+        return fallback_s
+
+    try:
+        delay_s = float(retry_after)
+    except (TypeError, ValueError):
+        try:
+            retry_at = parsedate_to_datetime(retry_after)
+        except (TypeError, ValueError, OverflowError):
+            return fallback_s
+        if retry_at.tzinfo is None:
+            retry_at = retry_at.replace(tzinfo=UTC)
+        delay_s = (retry_at - datetime.now(UTC)).total_seconds()
+
+    if not math.isfinite(delay_s):
+        return fallback_s
+    return max(0, math.ceil(delay_s))
+
+
 class OpenAIClient(LLMClient):
     """LLM client adapter for OpenAI chat completion models via openai SDK."""
 
@@ -25,25 +63,26 @@ def __init__(
         api_key: str | None = None,
         model: str = "gpt-4o",
         client: Any | None = None,
+        retry_after_fallback_s: int = 60,
     ) -> None:
         """Initialize the OpenAI client adapter.
 
         Args:
-            api_key: OpenAI API key. If omitted, uses OPENAI_API_KEY environment variable.
+            api_key: OpenAI API key supplied by application settings.
             model: OpenAI model identifier (default: gpt-4o).
             client: Optional pre-configured AsyncOpenAI client instance (useful for mocking).
+            retry_after_fallback_s: Fallback delay when a rate limit response has no header.
         """
         self.model = model
-        self.api_key = api_key or os.getenv("OPENAI_API_KEY")
+        self.api_key = api_key
+        self.retry_after_fallback_s = retry_after_fallback_s
 
         if client is not None:
             self._client = client
         elif self.api_key:
             self._client = openai.AsyncOpenAI(api_key=self.api_key)
         else:
-            raise AuthenticationError(
-                "OpenAI API key not configured. Set OPENAI_API_KEY environment variable or pass api_key."
-            )
+            raise AuthenticationError("OpenAI API key is not configured in application settings.")
 
     async def aclose(self) -> None:
         """Close the underlying AsyncOpenAI client and its connection pool."""
@@ -126,7 +165,8 @@ async def chat(
         except openai.AuthenticationError as exc:
             raise AuthenticationError(f"OpenAI authentication failed: {exc}") from exc
         except openai.RateLimitError as exc:
-            raise RateLimitExceededError(retry_after=60) from exc
+            retry_after = _retry_after_seconds(exc, self.retry_after_fallback_s)
+            raise RateLimitExceededError(retry_after=retry_after) from exc
         except openai.OpenAIError as exc:
             status_code = getattr(exc, "status_code", None)
             raise LLMProviderError(

docker-compose.yml

@@ -133,6 +133,7 @@ services:
       - LLM_MAX_RETRIES=${LLM_MAX_RETRIES:-3}
       - LLM_RETRY_BASE_DELAY_S=${LLM_RETRY_BASE_DELAY_S:-0.5}
       - LLM_RETRY_MAX_DELAY_S=${LLM_RETRY_MAX_DELAY_S:-10}
+      - RETRY_AFTER_FALLBACK_S=${RETRY_AFTER_FALLBACK_S:-60}
       # No default key. An unset or empty API_KEYS leaves the api service to fail at
       # startup on Settings validation rather than running with a published key.
       # Compose keeps ${VAR:-} so `down`, `ps` and `logs` still work without it.

docs/DEPLOY.md

@@ -217,6 +217,7 @@ API_KEYS=${API_KEYS}
 FILE_TOOL_BASE_DIR=/tmp/agentkit_sandbox
 RUN_TIMEOUT_S=60
 LLM_MAX_RETRIES=3
+RETRY_AFTER_FALLBACK_S=60
 ENVFILE
 
 chmod 600 /opt/agentkit/.env

docs/PROGRESS.md

@@ -80,6 +80,7 @@ This document tracks execution progress across all backlog tasks, including pull
 | ROADMAP-02 | ROADMAP | SQL Markdown Cell Escaping | `fix/roadmap-02-03-04-tools-hardening` | #120 | 1 | Done | Cells escaped (`\|`, `\n`, `\uXXXX` controls) and length-capped before markdown output; closes #94 |
 | ROADMAP-03 | ROADMAP | read_file Read-After-Stat Race | `fix/roadmap-02-03-04-tools-hardening` | #120 | 1 | Done | Byte cap enforced on a bounded `read(max_bytes + 1)` in one open; closes #95 |
 | ROADMAP-04 | ROADMAP | Calculator Non-Finite Result Guard | `fix/roadmap-02-03-04-tools-hardening` | #120 | 1 | Done | `math.isfinite` result check, inf literals rejected, pow pair guard named; closes #96 |
+| GH-92/111/113 | OPS | Operational Error Reporting | `fix/error-reporting-issues` | #123 | 6 | In Review | Invalid Postgres ports, sanitized agent failures and logged trace writes, settings-based OpenAI fallback, structured Gemini classification; addresses #92, #111 and item 5 of #113 |
 
 ---
 
@@ -515,3 +516,9 @@ This document tracks execution progress across all backlog tasks, including pull
 - **PR**: `fix/roadmap-02-03-04-tools-hardening` — PR #120 (three `fix(tools)` commits, squash-merged; docs rows in this commit)
 - **Commits**: 3 (+1 docs)
 - **Notes**: Closed the first three `ROADMAP` issues filed from the 2026-10-27 assessment. **#94** — `format_rows_as_markdown` now routes every cell (and column name) through `_format_cell`/`_escape_cell_text`: backslash first, then `|` → `\|`, newline/CR/tab → `\n`/`\r`/`\t`, every other C0/DEL control character → `\uXXXX`, and a rendered-length cap `MAX_CELL_CHARS` (200) with a `[Truncated]` marker, so a database value can no longer forge table rows, columns or prompt structure (stored prompt injection). **#95** — `read_sandboxed_file` enforces the byte cap on the bytes actually read (`read(max_bytes + 1)` from a single open) instead of a `stat().st_size` check + second open; oversize keeps the same user-visible phrase, an exact-cap file stays fully readable, and a regression test stands in for the race by lying about `st_size` (the old code returned the full oversized file). **#96** — `safe_calculate` rejects non-finite float results (`1e308 * 10 ** 3` no longer renders `inf` into the prompt) and non-finite float literals (`1e999` parses to inf) at the operand boundary; the `Pow` guard's bare `1000`/`100` literals became `MAX_POW_BASE_MAGNITUDE`/`MAX_POW_PAIR_EXPONENT` with unchanged values. 11 tests added (`test_tool_sql_readonly.py`, `test_tool_read_file.py`, `test_tool_calculator.py`); suite moved from 483 to 494 passing.
+
+### PR #123: Operational Error Reporting (#92 / #111 / #113 item 5)
+- **Status**: In Review
+- **PR**: https://github.com/getdownandcode/AgentKit/pull/123
+- **Commits**: 6 (three issue-specific fixes, initial progress bookkeeping, review fix, and this commit correcting the count)
+- **Notes**: **#92** — `audit_credentials()` now catches the `ValueError` raised by component-mode Postgres URL resolution for a malformed `POSTGRES_PORT`, reports one stable `POSTGRES_PORT is not a valid integer.` problem, and continues the audit without a traceback. **#111** — duplicate-tool failure reasons keep the tool name and loop category without echoing arguments; execution logs no longer include raw arguments; trace-sink errors are logged at `WARNING` with `run_id`/`step_no` while the run continues; OpenAI credentials come only from `Settings`, and rate-limit handling honors the `retry-after-ms` and `retry-after` response headers (numeric seconds or HTTP-date) before using the configured `RETRY_AFTER_FALLBACK_S` (default 60). A Codex review follow-up added regression coverage for header precedence, milliseconds conversion, malformed headers, fallback and HTTP-date parsing. **#113 item 5** — Gemini error classification inspects structured API status/reason fields and selects `code` only when non-null before falling back to `status_code`. Regression tests also cover malformed port input, sensitive failure reasons, trace logging, configured retry fallback, and structured Gemini errors. Verified: full `pytest` suite passes (real-infrastructure tier skipped because disabled), `ruff check .`, `ruff format --check .`, and `mypy agentkit` pass. The other six independent #113 findings remain outside this PR.

scripts/check_deploy_credentials.py

@@ -164,26 +164,36 @@ def audit_credentials(env: Mapping[str, str]) -> list[str]:
     # Resolved exactly the way Settings resolves them: an explicit URL wins, otherwise the
     # URL is assembled from the POSTGRES_* components with the credentials escaped. Auditing
     # the raw variables would either miss the component form or misread an escaped password.
-    app_url = (
-        resolve_postgres_url(
-            env,
-            url_var="DATABASE_URL",
-            user_var="POSTGRES_USER",
-            password_var="POSTGRES_PASSWORD",
-            default_user=DEFAULT_POSTGRES_USER,
+    try:
+        app_url = (
+            resolve_postgres_url(
+                env,
+                url_var="DATABASE_URL",
+                user_var="POSTGRES_USER",
+                password_var="POSTGRES_PASSWORD",
+                default_user=DEFAULT_POSTGRES_USER,
+            )
+            or ""
         )
-        or ""
-    )
-    readonly_url = (
-        resolve_postgres_url(
-            env,
-            url_var="DATABASE_READONLY_URL",
-            user_var="POSTGRES_READONLY_USER",
-            password_var="POSTGRES_READONLY_PASSWORD",
-            default_user=DEFAULT_READONLY_USER,
+    except ValueError:
+        app_url = ""
+        problems.append("POSTGRES_PORT is not a valid integer.")
+
+    try:
+        readonly_url = (
+            resolve_postgres_url(
+                env,
+                url_var="DATABASE_READONLY_URL",
+                user_var="POSTGRES_READONLY_USER",
+                password_var="POSTGRES_READONLY_PASSWORD",
+                default_user=DEFAULT_READONLY_USER,
+            )
+            or ""
         )
-        or ""
-    )
+    except ValueError:
+        readonly_url = ""
+        if "POSTGRES_PORT is not a valid integer." not in problems:
+            problems.append("POSTGRES_PORT is not a valid integer.")
     raw_redis = (env.get("REDIS_URL") or "").strip()
     redis_url, redis_problems = _resolve_redis_url(raw_redis, env.get("REDIS_PASSWORD"))
     problems.extend(redis_problems)

tests/unit/test_agent_loop_detector.py

@@ -60,6 +60,7 @@ def search_query(q: str) -> str:
     assert result.status == RunStatus.FAILED
     assert "duplicate tool call loop" in (result.failure_reason or "").lower()
     assert "search_query" in (result.failure_reason or "")
+    assert "python" not in (result.failure_reason or "")
     assert result.final_answer is None
 
 

tests/unit/test_deploy_credentials.py

@@ -174,6 +174,14 @@ def test_malformed_redis_url_is_reported_instead_of_raising(url: str) -> None:
     assert problems == ["REDIS_URL is not a valid connection URL."]
 
 
+def test_invalid_postgres_port_is_reported_without_traceback() -> None:
+    """Malformed component ports become one stable operator-facing problem."""
+    problems = audit_credentials(
+        _env(DATABASE_URL=None, DATABASE_READONLY_URL=None, POSTGRES_PORT="abc")
+    )
+    assert problems.count("POSTGRES_PORT is not a valid integer.") == 1
+
+
 def test_problem_messages_never_echo_the_secret() -> None:
     """The guard reports variable names, never the credential it rejected."""
     secret = "hunter2"

tests/unit/test_gemini_adapter.py

@@ -4,6 +4,7 @@
 
 import pytest
 
+from agentkit.core.errors import AuthenticationError, LLMProviderError
 from agentkit.core.types import Role
 from agentkit.llm.base import Message
 from agentkit.llm.gemini import GeminiClient
@@ -52,6 +53,39 @@ async def test_gemini_text_chat_translation(mock_genai_client: MagicMock) -> Non
     assert mock_genai_client.aio.models.generate_content.called
 
 
+@pytest.mark.asyncio
+async def test_gemini_classifies_structured_invalid_key_reason(
+    mock_genai_client: MagicMock,
+) -> None:
+    """Authentication classification reads provider error metadata, not display text."""
+    error = RuntimeError("request rejected")
+    error.code = 400  # type: ignore[attr-defined]
+    error.status_code = None  # type: ignore[attr-defined]
+    error.status = "INVALID_ARGUMENT"  # type: ignore[attr-defined]
+    error.details = {"error": {"reason": "API_KEY_INVALID"}}  # type: ignore[attr-defined]
+    mock_genai_client.aio.models.generate_content.side_effect = error
+    adapter = GeminiClient(api_key="fake-key", client=mock_genai_client)
+
+    with pytest.raises(AuthenticationError):
+        await adapter.chat([Message.user("hello")])
+
+
+@pytest.mark.asyncio
+async def test_gemini_status_code_falls_back_when_code_is_none(
+    mock_genai_client: MagicMock,
+) -> None:
+    """A present-but-empty code does not hide the alternate status_code field."""
+    error = RuntimeError("provider unavailable")
+    error.code = None  # type: ignore[attr-defined]
+    error.status_code = 503  # type: ignore[attr-defined]
+    mock_genai_client.aio.models.generate_content.side_effect = error
+    adapter = GeminiClient(api_key="fake-key", client=mock_genai_client)
+
+    with pytest.raises(LLMProviderError) as raised:
+        await adapter.chat([Message.user("hello")])
+    assert raised.value.status_code == 503
+
+
 @pytest.mark.asyncio
 async def test_gemini_tool_call_translation(mock_genai_client: MagicMock) -> None:
     """Verify function calling translation and tool call extraction."""

tests/unit/test_llm_factory.py

@@ -71,12 +71,14 @@ def test_create_from_settings_openai() -> None:
         LLM_MODEL="gpt-4o",
         OPENAI_API_KEY="test-openai-key",
         LLM_MAX_RETRIES=2,
+        RETRY_AFTER_FALLBACK_S=23,
     )
     client = create_llm_client_from_settings(settings)
     assert isinstance(client, RetryingLLMClient)
     assert isinstance(client._client, OpenAIClient)
     assert client.model == "gpt-4o"
     assert client._max_retries == 2
+    assert client._client.retry_after_fallback_s == 23
 
 
 def test_create_from_settings_zero_retries() -> None: