Skip to content

fix(gateway): preserve rate-limit failure metadata - #64686

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
AtakanGs:fix-gateway-rate-limit-session-state
Jul 28, 2026
Merged

fix(gateway): preserve rate-limit failure metadata#64686
teknium1 merged 1 commit into
NousResearch:mainfrom
AtakanGs:fix-gateway-rate-limit-session-state

Conversation

@AtakanGs

@AtakanGs AtakanGs commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Preserves failed and failure_reason metadata when a gateway agent run
returns a non-empty rate-limit error response.

On retry exhaustion, run_conversation() already returns failure metadata,
but GatewayRunner._run_agent_inner() dropped it on the non-empty
final_response path. The outer gateway could therefore treat the generated
429 text as a normal assistant response and persist it into the session
transcript.

This change forwards the failure metadata through that return path, allowing
the existing transient-failure handling to preserve the user turn without
storing the gateway-generated error as assistant output.

Regression coverage verifies that:

  • a non-empty 429 response retains its failure metadata;
  • the 429 text is not persisted as an assistant transcript entry;
  • the same session can switch to another provider and continue successfully
    without replaying the previous 429 error.

The Desktop/TUI route was reviewed separately. Current main already keeps
retry-exhaustion error text out of backend conversation history and rebuilds
the runtime during a provider switch, so this PR does not modify that path.

Related Issue

Closes #64446

Testing

  • 19 passed
  • Ruff checks passed
  • git diff --check passed

@alt-glitch alt-glitch added type/bug Something isn't working comp/gateway Gateway runner, session dispatch, delivery sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P2 Medium — degraded but workaround exists labels Jul 15, 2026

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

Overview

Gateway fix: preserves rate-limit failure metadata instead of dropping it, so callers can distinguish rate-limit errors from other failures. 226 additions.

Assessment

  • Correctness: Preserving rate-limit metadata is the right behavior for proper error handling downstream.
  • Security: No security-sensitive changes.
  • Debug artifacts: None.

Summary

Clean fix. LGTM.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused regression fix. The premise is confirmed on current main: gateway/run.py:19672 rebuilds the non-empty result without failed, while gateway/run.py:12224 only suppresses assistant-error transcript persistence when that flag reaches the outer handler. The change in 68766bf88450645de60089b9f3a309f38a3ba0a6 restores that contract and preserves the classified metadata emitted by agent/conversation_loop.py:4201-4208.

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit label Jul 16, 2026
@teknium1
teknium1 force-pushed the fix-gateway-rate-limit-session-state branch from 68766bf to d345462 Compare July 28, 2026 20:57
@teknium1
teknium1 merged commit 5423e26 into NousResearch:main Jul 28, 2026
38 checks passed
teknium1 added a commit that referenced this pull request Jul 29, 2026
…n path

Sibling of #64686: _run_agent's empty-final_response early return dropped
failure_reason (and #64686 only fixed the non-empty path), so downstream
consumers (TUI billing surface, transient-failure persistence) lost the
structured reason exactly when a failed run produced no text.

Also hardens the two BasePlatformAdapter identity checks (edit_message /
delete_message) with getattr so duck-typed adapters without the attribute
mean 'capability absent', not AttributeError — this was crashing the
send_progress_messages path for minimal adapters and test fakes.
teknium1 added a commit that referenced this pull request Jul 29, 2026
…n path

Sibling of #64686: _run_agent's empty-final_response early return dropped
failure_reason (and #64686 only fixed the non-empty path), so downstream
consumers (TUI billing surface, transient-failure persistence) lost the
structured reason exactly when a failed run produced no text.

Also hardens the two BasePlatformAdapter identity checks (edit_message /
delete_message) with getattr so duck-typed adapters without the attribute
mean 'capability absent', not AttributeError — this was crashing the
send_progress_messages path for minimal adapters and test fakes.
caozuohua pushed a commit to caozuohua/hermes-agent that referenced this pull request Jul 30, 2026
…n path

Sibling of NousResearch#64686: _run_agent's empty-final_response early return dropped
failure_reason (and NousResearch#64686 only fixed the non-empty path), so downstream
consumers (TUI billing surface, transient-failure persistence) lost the
structured reason exactly when a failed run produced no text.

Also hardens the two BasePlatformAdapter identity checks (edit_message /
delete_message) with getattr so duck-typed adapters without the attribute
mean 'capability absent', not AttributeError — this was crashing the
send_progress_messages path for minimal adapters and test fakes.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
…n path

Sibling of NousResearch#64686: _run_agent's empty-final_response early return dropped
failure_reason (and NousResearch#64686 only fixed the non-empty path), so downstream
consumers (TUI billing surface, transient-failure persistence) lost the
structured reason exactly when a failed run produced no text.

Also hardens the two BasePlatformAdapter identity checks (edit_message /
delete_message) with getattr so duck-typed adapters without the attribute
mean 'capability absent', not AttributeError — this was crashing the
send_progress_messages path for minimal adapters and test fakes.
33hodl pushed a commit to 33hodl/hermes-agent that referenced this pull request Aug 12, 2026
…n path

Sibling of NousResearch#64686: _run_agent's empty-final_response early return dropped
failure_reason (and NousResearch#64686 only fixed the non-empty path), so downstream
consumers (TUI billing surface, transient-failure persistence) lost the
structured reason exactly when a failed run produced no text.

Also hardens the two BasePlatformAdapter identity checks (edit_message /
delete_message) with getattr so duck-typed adapters without the attribute
mean 'capability absent', not AttributeError — this was crashing the
send_progress_messages path for minimal adapters and test fakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

# Bug: HTTP 429 error record persists in session context and cannot be cleared

4 participants