[getdownandcode]

Summary

Remediates 17 findings from a whole-project audit. The theme: several paths converted a misconfiguration or a transient fault into a working-looking success — HTTP 200 with fabricated data where there should have been a 503.

Rebased onto the AUD-01/AUD-04/AUD-05 follow-ups that landed while this branch was open; overlapping fixes were reconciled with main and the decisions recorded in ADR-009/010/011.

Closes #47 tracking M13-T06.

Critical

• Settings.API_KEYS no longer defaults to a key published in the repository (ak_test_key_12345). Validation rejects an empty or whitespace-only value, so a deployment without a .env fails at startup instead of serving unauthenticated traffic. The compose migrations service now receives API_KEYS too, since alembic and scripts/seed.py construct Settings.
• get_llm_client no longer falls back to FakeLLMClient. Provider construction failure raises ServiceUnavailableError (503). Previously a missing GEMINI_API_KEY produced HTTP 200 with scripted text. LLM_PROVIDER=fake remains an explicit opt-in resolved by the factory.
• The per-request Agent assembly's override is now explicit as app.state.agent_override, so the contract is distinguishable from the memoized first assembly removed in AUD-01.
• Retry policy is configurable end to end. AUD-04 wired RetryingLLMClient into create_llm_client_from_settings; this PR replaces the single MAX_RETRIES with LLM_MAX_RETRIES, LLM_RETRY_BASE_DELAY_S and LLM_RETRY_MAX_DELAY_S and passes all three through the factory. Deps stay thin and return the factory client unchanged, so the retry budget is never doubled.
• TieredMemoryStore forwards get_run_trace/list_runs. The trace endpoint duck-types on get_run_trace, so with a Tiered store it fell through to a trace sink that may hold no rows and returned an empty trace despite rows existing.
• http_fetch: post-connect SSRF verification. Validation resolved the host and checked the addresses, then let httpx resolve again — a short-TTL DNS answer could hand back a public address during validation and a loopback or 169.254.169.254 address during connection. The connected socket's server_addr is now checked against the validated addresses.
• http_fetch: no blocking DNS, bounded body. socket.getaddrinfo ran inline on the event loop; it moves to asyncio.to_thread. response.text buffered the whole body before slicing to max_chars; the body is now streamed under an explicit byte budget.
• Rate limiter fallback is atomic. The non-scripting path trimmed and counted in one transaction then inserted in a second. Measured against live Redis: the old version admitted 39 of 50 simultaneous requests against a limit of 5. The new WATCH/MULTI/EXEC version admits exactly 5. The AUD-10 in-memory fallback for Redis outages is untouched and still applies to RedisErrors.

Moderate

• Both LLM adapters wrap response parsing in the error boundary. Reading raw_response.text raises on a blocked prompt and an empty choices/candidates list raised IndexError, both escaping as raw SDK exceptions and breaking the error taxonomy.
• Malformed tool-call JSON is reported as such. It was rewritten to {"raw": ...}, which then failed argument validation with a misleading "missing required field" message.
• Full UUIDs for generated tool_call_ids. str(uuid4())[:8] is 32 bits and collision-prone within a long session; the id is the join key between an assistant tool call and its result.
• RetryingLLMClient.__getattr__ no longer recurses when _client was never assigned.
• /health returns fixed client-facing strings and logs the driver error. str(exc) from SQLAlchemy and Redis embeds host, port and database name, on an unauthenticated endpoint.
• sql_readonly: row limit applied in SQL. fetchmany(row_limit) alone still makes the database plan and execute the full scan. statement_timeout is now pushed to PostgreSQL. The keyword scan masks string literals and quoted identifiers first, so WHERE action = 'insert' is no longer rejected, and #>/#>>/#- are recognised as JSON path operators rather than comment markers.

Minor

• RedisMemoryStore.truncate_messages off-by-one. [system_msg, *non_system[-max_messages:]] returned max_messages + 1 entries whenever a system prompt was present, so the bound did not hold. max_messages is now a total budget, as the parameter name implies.
• Run goals no longer logged at INFO (length only, content at DEBUG) in both core/agent.py and api/routes.py.
• scripts/seed.py uses a statement-aware splitter handling string literals, line/block comments and dollar-quoted bodies, instead of split(";").
• Calculator bounds exponentiation by result bit length. The per-exponent cap still allowed (999**100)**5; every individual exponent stays within MAX_EXPONENT while the result reaches ~5000 bits.
• examples/demo.py requires --offline for scripted execution rather than silently downgrading on a missing key, and raises a SystemExit with a CLI-appropriate message. Offline runs also skip get_settings() entirely, so the demo works with no services and no .env.

Verification

Hermetic tier, post-rebase:

ruff check .            pass
ruff format --check .   pass (110 files)
mypy agentkit tests     pass (94 files)
pytest                  298 passed, 25 skipped
coverage                92.86% overall; >=92% on core/tools/llm

The real-infrastructure tier (AGENTKIT_REAL_INFRA=1) runs in CI's Test Against Live PostgreSQL and Redis job, including the new tests/real/test_redis_real.py regression test that asserts exactly 5 admissions from 50 concurrent calls to the non-scripting fallback. It was verified to fail against the old implementation.

New tests/unit/test_audit_regressions.py pins each fix, including the deps delegation contract (no double retry wrap), the CLI-strict demo behaviour, and the health-endpoint leak check.

Notes for review

• Breaking change: API_KEYS is now required everywhere. tests/conftest.py seeds a fixed value before agentkit.api.main is imported, since that module builds an app at import time. .env.example, README.md and docs/DEPLOY.md updated; CI already sets API_KEYS in both jobs.
• Breaking change (env standalone): MAX_RETRIES is renamed to LLM_MAX_RETRIES; the two delay settings are new.
• app.state.agent was renamed to app.state.agent_override to make the override intent explicit now that it is not a cache.
• The rebase kept AUD-01's strict dependency resolution and AUD-04's factory-level retry wrapping; the audit's original deps-level wrappers were dropped as superseded, and the regression tests were reconciled accordingly.
• The http_fetch tests drive AsyncClient.stream rather than AsyncClient.get, following the tool's move to streaming. Two existing tests asserted the buggy max_messages + 1 truncation and were corrected.
• Recorded as ADR-009 (fail-closed configuration), ADR-010 (post-connect SSRF verification) and ADR-011 (atomic fallbacks and domain-error boundaries).