Skip to content

fix(mcp): a usage-limit 429 after the run is no longer reported as agent_busy (#3244) - #3499

Merged
vybe merged 3 commits into
devfrom
feature/3244-post-run-429-not-busy
Oct 10, 2026
Merged

vybe merged 3 commits into
devfrom
feature/3244-post-run-429-not-busy

Conversation

@trinity-ability

Copy link
Copy Markdown
Contributor

Description

TrinityClient.chat() answered every HTTP 429 from POST /api/agents/{name}/chat as "queue full", and chat_with_agent reported it as agent_busy — "Please wait N seconds before retrying". Two different 429s reached that branch. The admission refusal (X-Trinity-Error-Code: capacity) dispatched nothing. The usage / rate-limit 429 is raised after the agent ran the turn and the row was written failed, so a retry repeats whatever the partial run already did. That second 429 also carried nothing naming its execution.

  • Backend (chat_execution_service.py): failures raised from _finalize_http_failure now carry an additive X-Trinity-Execution-Id response header beside the error code. Response bodies are unchanged.
  • MCP client: only a 429 with code capacity is agent_busy. Any other 429 is a new rate_limited result with retryable: false.
  • Delegation contract (both copies): the agent_busy sentence now says nothing ran and to re-send after retry_after_seconds. This changes the prompt every agent receives (6 characters shorter, inside the budget).
  • Docs that stated the old behaviour.

The post-run result:

{ "status": "rate_limited", "agent": "target", "code": "billing",
  "execution_id": "<failed row>", "error": "<agent's own error text>",
  "retryable": false,
  "message": "The run on 'target' started and then failed on a usage or rate limit; it is recorded as failed. Part of the work may already be done, so do not re-send this message as it is. Read what happened with get_execution_result(...) and decide from that." }

A 429 with no error code (a proxy, or a backend older than #2919) is also rate_limited, without an execution_id, pointing at list_recent_executions. "Nothing ran, retry" is the one label that is unsafe when wrong.

For the reviewer:

  • X-Trinity-Execution-Id already exists as a request header meaning the caller's own turn. On a response it now means the turn the response is about. Say if a distinct name is preferred.
  • The connector tools (run_playbook, ask) call chat() and pass the result through, so a connector caller can now receive rate_limited, whose message names get_execution_result / list_recent_executions — tools that scope may not have. No connector-specific wording was added.
  • client.task() is unchanged: it throws API error (429) and never claimed nothing ran; a billing failure on /task answers 503. fan_out raises no 429.

Related Issue

Fixes #3244

Journey Impact

Journey Impact: none: bug fix to the label on an existing error result; no journey promise is added or changed

Type of Change

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

Testing

  • I have tested this locally

  • New tests added (if applicable)

  • All existing tests pass

  • Every new test executes the changed path

  • src/mcp-server/src/chat-429-split.test.ts (8 tests) drives both 429 shapes through the real chat() and the real chat_with_agent tool and asserts the answers differ.

  • tests/unit/test_3244_post_run_429_names_execution.py (7 tests) drives the real finalizer.

  • MCP server npm test: 833 pass, 0 fail; tsc --noEmit clean. 73 neighbouring Python unit files: 2363 passed, 4 skipped, 7 xfailed.

  • Not run: the full tests/unit suite, and no live backend — the header is verified through the finalizer and a FastAPI HTTPException round-trip, not the full route.

  • Two existing fixtures that used an unmarked 429 to mean "busy" (chat-gate.test.ts, chat-parent-execution.test.ts) now send the capacity header.

Mutation: with client.ts and tools/chat.ts reverted, 7 of the 8 MCP tests go red; with the header removed from _apply_sub003_autoswitch, 5 of the 7 Python tests go red. Restored byte-identical.

Checklist

  • My code follows the project's style guidelines
  • I have updated the documentation (if applicable)
  • I have not committed any sensitive data (API keys, credentials, etc.)

🤖 Generated with Claude Code

trinity-ability and others added 3 commits October 9, 2026 23:52
…ent_busy (#3244)

Root cause: TrinityClient.chat() turned every 429 from POST /chat that the
gate parser did not claim into queue_status "queue_full", and runAgentChat
answered it agent_busy ("Please wait N seconds before retrying"). Two
different 429s reach that branch. The admission refusal
(X-Trinity-Error-Code: capacity) dispatched nothing. The usage / rate-limit
429 is raised by _finalize_http_failure -> _apply_sub003_autoswitch after the
agent ran the turn and the row was written failed, so a retry repeats what
the partial run did. That 429 also carried nothing naming its row.

Fix:
- Backend: the failures raised from _finalize_http_failure name their row on
  an additive X-Trinity-Execution-Id response header, beside
  X-Trinity-Error-Code (the #2889 shape; bodies unchanged, so the frontend
  and every other reader of that 429 see what they saw).
- MCP client: only a 429 with code "capacity" is agent_busy. Any other 429 is
  a rate_limited result, retryable: false, which names the failed execution
  and points at get_execution_result; with no execution id it says the run
  may have started and points at list_recent_executions. A 429 with no code
  fails the same way: "nothing ran, retry" is the one answer that is unsafe
  when wrong. The backend's own "Please retry." is not passed on.
- Delegation contract (both copies): agent_busy now means nothing ran.
- Docs that stated the old behaviour.

Not changed: client.task() still throws "API error (429)" for /task. It never
claimed that nothing ran, and a billing failure on /task answers 503.

Tests: src/mcp-server/src/chat-429-split.test.ts drives both 429 shapes
through the real chat() and the real chat_with_agent tool;
tests/unit/test_3244_post_run_429_names_execution.py drives the real
finalizer. Mutation: with client.ts and tools/chat.ts reverted, 7 of the 8
MCP tests go red; with the header removed from _apply_sub003_autoswitch, 5 of
the 7 Python tests go red. Restored byte-identical, all green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…429-not-busy

# Conflicts:
#	src/backend/services/chat_execution_service.py
Mechanical, per the merge-train note on the PR. #3486 (#3470) landed on
dev after this branch forked: its exhausted-walk 429 raises with
`auto_switch: walk.summary()`, a non-empty dict whose `switched` is false,
so `Boolean(auto_switch)` would append "The agent's subscription was
switched after the failure." when nothing was switched. Read the flag when
the body carries it; keep presence as the fallback for older bodies.
mcp-server: 836/836, tsc clean.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@vybe

vybe commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

merge-train (2026-10-10): on this train. Two commits pushed to this branch, both mechanical:

  1. 858e1b873 — merge of dev, resolving the one source conflict with fix(sub-003): a turn walks every subscription and the API key before it fails (#3470) #3486 (bug: Workspace turn fails with "reached its usage limit" instead of walking every subscription — SUB-003 retries once, then hands the retry to the user #3470) in src/backend/services/chat_execution_service.py: _apply_sub003_autoswitch now takes both execution_id= (this PR) and the keyword-only walk= (dev), the docstring keeps both paragraphs, and _finalize_http_failure passes both. Side effect worth knowing: dev's exhausted-walk 429 now also carries X-Trinity-Execution-Id, since it raises through the same code_headers. Verified on the merged tree: test_3244_*, test_3470_*, test_ent568_delegation_contract, test_1483_*, test_2433_*, test_ent279_* → 162 passed, 1 skipped.
  2. a15d1cb79 — src/mcp-server/src/client.ts parseRateLimitedRun reads auto_switch.switched when the body carries it, falling back to presence only for older bodies. Only observable after fix(sub-003): a turn walks every subscription and the API key before it fails (#3470) #3486: its walk summary is a non-empty dict with switched: false, so Boolean(auto_switch) would have appended "The agent's subscription was switched after the failure." when nothing was switched. mcp-server 836/836, tsc clean.

Validation (/validate-pr + /review) found no CRITICAL; both halves are executed by their tests (real _finalize_http_failure asserting the exact header names client.ts reads; real TrinityClient + chat_with_agent.execute with only fetch stubbed). Open for the author's judgment, not blockers: the request/response overload of the X-Trinity-Execution-Id header name, and connector-scope callers receiving a rate_limited message that names tools their scope lacks (pre-existing pattern).

@vybe vybe left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge-train: batch validated on train/20261010-0851 (#3508)

@vybe
vybe merged commit 6434ff9 into dev Oct 10, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants