fix(classifier): retry provider-injected parameter 400s instead of aborting the turn

[troyrowe-resource]

What does this PR do?

Stops a turn being aborted when the provider rejects a request parameter that Hermes never sent.

The Codex OAuth backend (chatgpt.com/backend-api/codex) intermittently injects prompt_cache_retention into its own upstream call and then rejects it with HTTP 400 invalid_parameter. Hermes only sets that field for api.meta.ai and bedrock-mantle.*.api.aws hosts (agent/transports/codex.py::_default_prompt_cache_retention_for_request) — never for the Codex route. The failing request body confirms it: no prompt_cache_retention key present.

_classify_400 fell through to its catch-all and returned format_error / retryable=False, which trips the is_client_error abort gate in agent/conversation_loop.py (~line 6142) and kills the turn on the first attempt. format_error is right for a deterministic bad parameter, but this rejection is random: a byte-identical retry succeeds ~80% of the time. The cost of getting it wrong is high because the discarded requests are large-context ones.

This classifies the case as a retryable server_error so the existing retry loop absorbs it, and keeps genuine client-side bad-parameter 400s failing fast exactly as before.

Deliberately narrow, to avoid turning a useful fast-fail into a retry flood:

• keyed on a small map of parameters Hermes only sends on specific routes (_SERVER_INJECTED_PARAM_SENDERS);
• requires the message to actually be a rejection of that parameter, not an incidental mention;
• skipped when the current provider is one that legitimately sends the field, so a real request error there still fails fast;
• should_compress=False — the request shape was never the problem, so this must not enter the compression loop.

Related Issue

Fixes #90257

Type of Change

• 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

• agent/error_classifier.py
  • added _SERVER_INJECTED_PARAM_SENDERS + _is_server_injected_param_rejection() helper
  • _classify_400: new branch before the request-validation branch returning retryable server_error
  • _classify_by_status (500/502 branch): guard added so a proxy surfacing the same injected-parameter rejection as 5xx keeps the generic retryable-5xx handling, instead of being fast-failed as format_error
• tests/agent/test_error_classifier.py
  • added TestServerInjectedParameterRejection (10 tests, incl. 4 boundary/regression guards)

How to Test

Reproduction (what made this diagnosable) — a minimal request to the Codex endpoint carrying no cache parameters at all:

body = {"model": "gpt-5.6-sol", "instructions": "t",
        "input": [{"role": "user", "content": "OK"}],
        "store": False, "stream": True}

n=20 sequential: 4 failed (20%) with prompt_cache_retention is not supported on this model. n=12 concurrent: 0 failed. Random, and on a request that cannot contain the parameter.

Verifying the fix:

1. pytest tests/agent/test_error_classifier.py -q → 107 passed
2. Red-green verified: with the new branch disabled (if False and _is_...), the 4 positive tests fail with format_error != server_error; re-enabled, all 10 pass. The 4 boundary tests pass either way, so they can't mask a regression.
3. Abort-gate check — replicating the is_client_error expression from conversation_loop.py against the real classification yields False, i.e. the loop retries rather than aborting.
4. Live end-to-end against the real Codex API: injected the exact BadRequestError on attempt 1, let the real retry policy handle it, recovered on attempt 2, model returned the expected RETRY_OK.

Boundary cases held (all asserted in tests):

Case	Verdict
Codex retention 400 (envelope + bare detail body)	server_error, retryable
Same on a small 1-message request	server_error, retryable
Same surfaced as 500/502 by a proxy	server_error, retryable
Retention 400 from meta-ai / bedrock-mantle (we do send it there)	format_error, fast-fail
max_tokens unsupported-parameter 400	format_error, fast-fail
unknown_parameter 502 (the codex.nekos.me case)	format_error, fast-fail
Genuine context-overflow 400	context_overflow, compress

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(classifier): ...)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix (2 files)
• I've run the test suite and all tests pass — see note below
• I've added tests for my changes
• I've tested on my platform: Ubuntu 24.04, Python 3.11.15

Test note: verified in a clean worktree checked out from origin/main (9162ea6db) with the change cherry-picked, so no local config or plugins are in play.

• tests/agent/test_error_classifier.py → 107 passed
• Full tests/agent/ tests/run_agent/ → 231 failed, 6296 passed, 31 skipped

Those 231 failures are pre-existing on main and unrelated to this change. I ran the identical suite against a pristine origin/main worktree with the change absent and diffed the failure sets:

baseline (pristine origin/main) failures : 231
with my change                  failures : 231
NEW failures introduced by my change     : 0
failures my change FIXED                 : 0
-> identical failure sets

The delta is exactly +10 passed, which is the 10 tests added here. The pre-existing failures cluster in test_models_dev.py (41), test_codex_app_server_integration.py (17), test_title_generator.py (15) and ~40 other files, and appear environmental (mock-call assertions in a sandbox without provider credentials) — happy to share the log if useful.

Documentation & Housekeeping

• I've updated relevant documentation — N/A (internal classification behaviour; the reasoning is captured in code comments at both branch sites)
• I've updated cli-config.yaml.example if I added/changed config keys — N/A (no config keys)
• I've updated CONTRIBUTING.md / AGENTS.md — N/A (no architecture or workflow change)
• I've considered cross-platform impact — N/A (pure string/dict logic, no platform-specific behaviour)
• I've updated tool descriptions/schemas — N/A (no tool behaviour change)

Screenshots / Logs

Before (turn dies on first attempt):

WARNING agent.conversation_loop: API call failed (attempt 1/3) error_type=BadRequestError
  provider=openai-codex base_url=https://chatgpt.com/backend-api/codex model=gpt-5.6-sol
  summary=HTTP 400: prompt_cache_retention is not supported on this model
ERROR   agent.conversation_loop: Non-retryable client error: Error code: 400 - {'error':
  {'message': 'prompt_cache_retention is not supported on this model', 'type':
  'invalid_request_error', 'param': 'prompt_cache_retention', 'code': 'invalid_parameter'}}

Two sessions hit this 25 seconds apart, each carrying ~410-550k tokens of context — a full large-context input discarded per abort, for a fault that clears on retry.

After (same error injected, retry loop recovers):

[1] classification: server_error retryable=True OK
[2] abort gate: not triggered -> loop retries OK
    attempt 1: InjectedBadRequest -> server_error retryable=True
[3] recovered on attempt 2; live model said: 'RETRY_OK'

[kshitijk4poor]

Merged via #91643. Your commits cherry-picked with authorship preserved.

Your fix is exactly right — the classification of server-injected parameter 400s as retryable server_error instead of non-retryable format_error is the correct approach, and the boundary tests (meta-ai sender stays non-retryable, max_tokens stays non-retryable, codex.nekos.me 502 stays non-retryable) are excellent.

Thanks for the thorough reproduction and analysis — the ~20% failure rate measurement on a minimal request that provably carried no cache parameters was key to confirming this is a server-side injection, not a client-side request shape issue.