fix(tests): classify a model-turn 503 instead of skipping on it (#2889) - #2894
Merged
Merged
Conversation
A 503 from POST /chat or /task was skipped as "Agent server not ready" at ~50 live-test sites, so an exhausted credit balance — the agent ran the turn and failed on the credential — read as a benign readiness skip while the run stayed green. The two causes shared one reason string because the sync 503 body is prose reconstructed from the agent's error text; the backend already computed TaskExecutionErrorCode and dropped it at _map_task_failure. Backend: an ADDITIVE X-Trinity-Error-Code header on every sync /chat|/task failure the backend classifies (auth / billing / network / agent_error / capacity / timeout). /task threads TaskExecutionResult.error_code; /chat derives it from agent_status_code with the same producer-side rule task_execution_service applies. Bodies are byte-identical to before. Tests: tests/testkit/readiness.py holds the one classifier — require_agent_answer reads the header first, falls back to transport vocabulary pinned to the backend source, and FAILS on anything the agent answered; only `network` skips, naming its evidence. A `requires_model` marker opts a test into the session model_provider_preflight fixture (claude_auth_configured flag, then one real /task probe on TEST_AGENT_NAME), so a stack whose credential cannot execute fails once with one cause; a credential-class verdict seen mid-session fails later model turns at setup instead of spending a 120 s call each. 53 sites converted across the six files in the issue plus test_fan_out.py and test_dynamic_thinking_status.py. An AST guard fails CI on a bare `if status == 503: pytest.skip` after a model-turn POST. Fixes #2889 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Regression details (head_sha: `340a818bf8ec64abbbf9b0f9d00d435a565f1566`)Seed Backend unit-suite regression diffPer-XML totals
❌ New failures introduced by HEAD (2)Tests failing under HEAD that did not fail under BASE in any seed:
🟢 Failures fixed by HEAD (2)
Legend: [F] = assertion failure, [E] = collection or fixture error. Reproduce locally: |
…dering # Conflicts: # docs/memory/learnings.md
Contributor
|
merge-train:
Validation came back READY with no critical findings. Worth recording what was checked, since this PR's whole subject is whether a test really runs:
I've filed #2919 for the gap this doesn't reach: 18 bare Merging once the required checks come back green on this push. |
vybe
approved these changes
Sep 20, 2026
vybe
left a comment
Contributor
There was a problem hiding this comment.
merge-train: validated 2026-09-20. Coverage mutation-proven on both halves (headers=None → 8 red; reverting one skip site → AST guard red). dev merged for the learnings.md append collision, keeping both sides. One regression diff red was a timing flake in test_subprocess_pgroup (unrelated file, 1 of 3 seeds); a full re-run with regenerated artifacts is green. Follow-up for the 429 twin filed as #2919.
vybe
pushed a commit
that referenced
this pull request
Sep 23, 2026
…skips; the admission 429s say "capacity" (#2919) (#2977) * test(readiness): red — a billing 429 must fail and a capacity 429 must skip (#2919) Replaces test_non_503_is_a_no_op (which pinned 429 as a no-op) with the three consumer cases the 429 arm has to satisfy. Red for the behaviour: DID NOT RAISE Failed / Skipped, 4 failed, 39 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(readiness): classify a model-turn 429 — billing fails, capacity skips, unknown fails (#2919) The 429 arm in require_agent_answer, mirroring #2894 one status code over: header first (billing/auth → fail + session cascade; capacity → skip with evidence, never recorded), X-Circuit-Open second, then on a 429 only the source-pinned capacity wording (transport vocabulary is not consulted), else unknown ⇒ FAIL. Verdict gains a trailing-default status; the reason builders name it; capacity_skip_reason is new and stays off the skip-audit allowlist. Gate: 63 → 89 passed. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(chat): the three admission-refusal 429s carry X-Trinity-Error-Code: capacity (#2919) TaskExecutionErrorCode.CAPACITY had zero assignment sites, so a queue-full 429 was recognisable only by the absence of a header plus its prose, while execution.md and the A2A feature flow already listed `capacity` as emitted. Three additive headers= kwargs, bodies byte-identical: - routers/chat.py: the /chat CapacityFull admission 429 (through chat_execution_service._error_code_headers, the one emitter) - _dispatch_async: the CapacityFull (capacity + backlog full) 429 - _map_task_failure: the at-capacity branch decides the status by the result text, so the code it sends now agrees (capacity overrides any result code; none exists on that branch today) task_execution_service.py untouched. Red (backend reverted from scratch copy): 3 failed, 24 passed — assert None == {'X-Trinity-Error-Code': 'capacity'} ×3 Green: test_2889_readiness_classifier + test_2889_error_code_header 92 passed; characterization suites (chat_admission, 946, 2661, 1483) 48 passed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(readiness): the AST guard covers == 429; delete the 18 dead 429 skips (#2919) _is_503_check → _is_laundered_status_check, keyed on readiness.LAUNDERED_STATUSES (one home for the vocabulary): `==`, `in` list/tuple/set, and an `or` of such checks. Known escapes are written down in its docstring (same limits #2894 accepted). Adds the matcher table and a walk meta-test pinning the four site files inside the guard. Guard widened BEFORE deleting — red listed exactly 18 offenders: test_dynamic_thinking_status.py:203,230,268,334,425,507 test_parallel_task.py:569,630,664,719,794,839 test_agent_chat.py:42,67,432,469,505 agent_server/test_agent_chat_direct.py:35 Each followed require_agent_answer(...), which now skips a capacity 429 and fails a billing/unknown one, so every branch was dead. Deleted; the downstream assert_status(…, 200) is unchanged. Mutation check: restoring test_agent_chat_direct.py's site → guard red with exactly 1 offender. Gate: 104 passed. The 18 integration sites were not run live (Docker down); the four files collect (98 tests). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs: a model turn's 429 is classified too; capacity is now emitted (#2919) - execution.md (header contract's owning area file): the three admission-refusal emitters of `capacity`, and the 429 arm of require_agent_answer; the value list there is now true. - agent-to-agent-collaboration.md: a queue-full 429 carries `capacity`. - STRATEGY.md / tests/README.md: "503 or 429" — capacity skips (unallowlisted), billing fails and arms the session fast-fail, the guard covers `== 429`. Requirements delta: none (no area covers the test harness or the header). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * docs(feature-flows): the queue-full 429 carries X-Trinity-Error-Code: capacity (#2919) /sync-feature-flows over origin/dev..HEAD: routers/chat.py and chat_execution_service.py map to execution-queue.md (the 429 body it documents) and agent-to-agent-collaboration.md (already updated). One sentence; no new flow, no index change. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(readiness): a fail-expecting case must go red on a skip, not report skipped (#2919) `Skipped` is not a `Failed`, so `pytest.raises(pytest.fail.Exception)` lets a wrongly-skipping helper propagate its skip: mutating require_agent_answer to "skip every 429" (the defect this issue removes) left the unit file at 71 passed / 6 skipped / 0 failed. Route every fail-expecting case through `_must_fail()`, which turns a skip into an AssertionError. Same mutation now: 6 failed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * test(readiness): keep the laundering guard operator-agnostic, as #2894's was (#2919) `_is_503_check` matched any comparison against 503; the widened matcher accepted only `==`/`in`, silently unguarding `>= 503`, `!= 503` and `not in [503]` on the 503 half. Match the constant regardless of operator (a strict superset of both), pin `>= 429` / `!= 503` / `not in [503]` in the shape table, and correct the "known escapes" list. Mutation: re-adding a `status_code >= 429: pytest.skip` site turns the guard red naming it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(chat): `capacity` fills an absent task code, never overrides the producer's (#2919) `_map_task_failure` picks its 429 by substring ("at capacity") on the result text and was stamping `capacity` over any code the result carried. That code is the producer's structured verdict on a turn that RAN (#2638 `billing`, whose `error` is the agent's own prose) — overriding it would let the new 429 arm SKIP an exhausted subscription whose prose happened to say "at capacity". Prose must not beat the structured code: `capacity` now only fills an absent code (the capacity rejection sets none, so every live at-capacity 429 still carries `capacity`). Red first: `{'X-Trinity-Error-Code': 'capacity'} != {'X-Trinity-Error-Code': 'billing'}`. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * merge-train: capacity site is _acquire_task_capacity; backlog path gets capacity on at-capacity (#2919) — mechanical, doc/comment accuracy Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: trinity-ability <309458136+trinity-ability@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /chat//taskwaspytest.skipped as "Agent server not ready" at ~50 live-test sites, so an exhausted credit balance (the turn ran and failed on the credential) read as a benign readiness skip while the run stayed green.X-Trinity-Error-Codeheader on every sync/chat|/taskfailure the backend classifies (auth/billing/network/agent_error/capacity/timeout)./taskthreads theTaskExecutionResult.error_codeit already computed and dropped at_map_task_failure;/chatderives it fromagent_status_codewith the same producer-side ruletask_execution_serviceapplies. Bodies are byte-identical — nothing parsing them breaks. No new substring classifier in the test tree (the bug: agent container OOM cascades into backend worker saturation and freezes admin UI #904 drift class).tests/testkit/readiness.py—require_agent_answerreads the header first, falls back to transport vocabulary pinned to the backend source, and fails on anything the agent answered; onlynetworkskips, naming its evidence (still unallowlisted, sorun-full.sh's audit keeps flagging it). Arequires_modelmarker opts a test into the sessionmodel_provider_preflightfixture (claude_auth_configuredflag, then one real/taskprobe onTEST_AGENT_NAME) so a stack whose credential cannot execute fails once with one cause; a credential-class verdict seen mid-session fails later model turns at setup instead of spending a 120 s call each (anunknown/agent_error503 fails only its own test — no cascade from one OOM). 53 sites converted across the six files in the issue plustest_fan_out.pyandtest_dynamic_thinking_status.py.Changes
src/backend/services/chat_execution_service.py—ERROR_CODE_HEADER,_error_code_headers,_classify_agent_http_failure; header threaded through_apply_sub003_autoswitch+_map_task_failuretests/testing_utils/readiness.py(via thetestkitsymlink) — classifier +require_agent_answer+ session verdicttests/conftest.py—requires_model→model_provider_preflightinjection,pytest_runtest_setupfast-fail;pyproject.tomlmarkerrequire_agent_answer(...)+@pytest.mark.requires_modeltests/unit/test_2889_readiness_classifier.py— classification table, fail/skip behaviour, backend-source contract for the transport vocabulary, AST guard (no bare 503 skip after a model-turn POST; every helper site is marked)tests/unit/test_2889_error_code_header.py— helper + both backend pathsarchitecture/execution.md,tests/README.md,docs/testing/STRATEGY.md, a2a flow error table,learnings.md, CSO diff reportAcceptance criteria
require_agent_answer→pytest.fail, code from the header; unknown ⇒ fail too)network/ transport vocabulary → "agent server still starting — … evidence: …")model_provider_preflight, cached session-fixture error +pytest_runtest_setup)tests/testkit/readiness.py), guarded by ASTTest Plan
cd tests && pytest unit/test_2889_readiness_classifier.py unit/test_2889_error_code_header.py -v— 63 passed; AST guard mutation-checked (reverting one site turns it red)test_736_a2a_outbound_edgesC1b are pre-existing on a clean tree (a2a untouched)lint_sys_modules.py/lint_root_test_placement.py— no new violations--setup-planshowsmodel_provider_preflightinjected intorequires_modelitemstest_agent_chat.py::test_send_message_returns_responsepasses against a local stack (preflight ran)/verify-localor a deploy)Out of scope: ~170 "Agent server not ready" skips on non-model proxies (files/git/dashboard) — a 503 there is the agent-server hop, never the provider.
Fixes #2889
🤖 Generated with Claude Code