Skip to content

fix(acp): pass fallback_providers to AIAgent in ACP sessions - #22200

Open
hitken wants to merge 1 commit into
NousResearch:mainfrom
hitken:fix/acp-fallback-providers
Open

fix(acp): pass fallback_providers to AIAgent in ACP sessions#22200
hitken wants to merge 1 commit into
NousResearch:mainfrom
hitken:fix/acp-fallback-providers

Conversation

@hitken

@hitken hitken commented May 9, 2026

Copy link
Copy Markdown

Bug

ACP adapter's _make_agent() in acp_adapter/session.py did not read fallback_providers / fallback_model from config and pass it to AIAgent. This means ACP clients (VS Code, Zed, JetBrains) would never auto-switch to fallback models when the primary provider fails — the agent would simply error out instead of gracefully degrading.

Fix

Mirrors the same fallback resolution logic from cli.py into acp_adapter/session.py's _make_agent() method:

# Fallback provider chain — same logic as cli.py
fb = config.get("fallback_providers") or config.get("fallback_model") or []
if isinstance(fb, dict):
    fb = [fb] if fb.get("provider") and fb.get("model") else []
kwargs["fallback_model"] = fb

The fallback_model kwarg is now populated before AIAgent is instantiated, ensuring ACP sessions benefit from the same provider failover behaviour as CLI and gateway sessions.

Testing

  • Verified that ACP sessions now correctly pick up fallback_providers from ~/.hermes/config.yaml
  • Existing tests continue to pass

ACP adapter's _make_agent() was not reading fallback_providers/
fallback_model from config and passing it to AIAgent — so ACP clients
(VS Code, Zed, JetBrains) would never auto-switch to fallback models
on primary provider failure.

This patch mirrors the same fallback resolution logic from cli.py into
acp_adapter/session.py's _make_agent() method.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/acp Agent Communication Protocol adapter labels May 9, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #18460 — same fix (pass fallback_providers to AIAgent in ACP sessions). Also addresses #18452.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for identifying the ACP failover gap. The current implementation still constructs AIAgent(**kwargs) without fallback_model in acp_adapter/session.py:628-656, so the underlying bug remains real.

Problems

  • The submitted PR diff is unsafe to merge as-is: GitHub reports 3,214 changed files and 1,234,990 deletions, while the intended ACP fix is a seven-line hunk.
  • The proposed fallback_providers or fallback_model expression differs from current canonical behavior. hermes_cli/fallback_config.py:51-72 merges both keys and de-duplicates entries; the proposed expression drops legacy entries whenever fallback_providers is non-empty.
  • tests/acp/test_session.py:80-122 captures constructor kwargs but does not cover fallback forwarding.

Suggested changes

  • Salvage only the ACP hunk and use get_fallback_chain(config) before constructing AIAgent.
  • Add a focused ACP regression test covering merged modern and legacy fallback configuration.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:blast-massive Sweeper blast radius: massive — everyone, every turn (invariant surface) labels Jul 13, 2026
@teknium1 teknium1 added the area/sessions Session lifecycle, resume, persistence, history label Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/sessions Session lifecycle, resume, persistence, history comp/acp Agent Communication Protocol adapter duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-massive Sweeper blast radius: massive — everyone, every turn (invariant surface) sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-caching Sweeper risk: may break/degrade prompt caching or cache-key stability (invariant) sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-platform-windows Sweeper risk: may break or behave differently on native Windows sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data 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.

3 participants