Skip to content

fix(acp): pass fallback_model chain to ACP sessions - #64868

Open
tzy-17 wants to merge 1 commit into
NousResearch:mainfrom
tzy-17:fix/acp-fallback-model-64441
Open

fix(acp): pass fallback_model chain to ACP sessions#64868
tzy-17 wants to merge 1 commit into
NousResearch:mainfrom
tzy-17:fix/acp-fallback-model-64441

Conversation

@tzy-17

@tzy-17 tzy-17 commented Jul 15, 2026

Copy link
Copy Markdown

Problem

ACP (editor) sessions built via _make_agent constructed AIAgent without the fallback_model kwarg, unlike the CLI (cli.py) and cron (scheduler.py) paths, which both wire get_fallback_chain(config).

As a result, ACP sessions silently ignored the configured fallback_providers / fallback_model chain — when the primary provider hit a rate-limit, overload, or connection failure, the session could never fall back to a backup provider.

Closes #64441

Fix

Pass fallback_model=get_fallback_chain(config) or None in acp_adapter/session.py _make_agent, matching the other agent-construction paths.

Verification

  • ruff check on touched files: clean
  • python -m py_compile: clean
  • Added a regression test (test_acp_real_agent_gets_fallback_chain_from_config) mirroring the existing test_acp_real_agent_gets_session_db_for_recall pattern, asserting the fallback chain reaches AIAgent.

Checklist

  • Change is minimal and scoped to the bug
  • Tests added
  • Lint passes

ACP session manager's `_make_agent` built `AIAgent` without the
`fallback_model` kwarg, unlike the CLI (`cli.py`) and cron
(`scheduler.py`) paths which both wire `get_fallback_chain(config)`.
As a result ACP (editor) sessions could never fall back to a backup
provider when the primary hit a rate-limit, overload, or connection
failure — the configured `fallback_providers`/`fallback_model` chain
was silently ignored.

Pass `fallback_model=get_fallback_chain(config) or None` to match the
other agent-construction paths.

Closes NousResearch#64441
@alt-glitch alt-glitch added type/bug Something isn't working comp/acp Agent Communication Protocol adapter P4 Best-effort: we will get to it when we get to it (no commitment) duplicate This issue or pull request already exists labels Jul 15, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #18460 — same code site (acp_adapter/session.py _make_agent) and same mechanism (missing fallback_model kwarg to AIAgent). #18460 is the earliest still-open fix PR in this cluster (also #22200, #36720). Marking as duplicate; the maintainer will pick the canonical PR to merge. related_to the underlying issues #64441 / #18452.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users area/sessions Session lifecycle, resume, persistence, history labels Jul 16, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Five of the 11 listed PRs address the ACP fallback defect: #18460, #22200, #36720, #57418, and #64868; their changes range from canonical get_fallback_chain(config) forwarding with tests to incorrect or unsafe variants. The other six concern Discord typing retries, diary skills, attribution cleanup, or Discord regression coverage rather than the reported ACP cause.

Related pull requests

Duplicates

#18460, #22200, #36720, #57418, and #64868 form the ACP fallback-propagation duplicate cluster, with #57418 already closed in favor of #18460; #22398 and merged #29671 represent the same Discord typing-retry implementation, while #28363 and #28616 overlap in stated daily-diary scope.

Suggested consolidation

Keep #18460 open with a salvage path: preserve its focused get_fallback_chain(config) wiring and combined-key regression coverage, consistent with both its recorded best-fix designation and COMMENTED keep_open review. Close #22200 as a duplicate of #18460 despite its keep_open verdict because its aggregate diff and or-based semantics are superseded there; close #36720 and #64868 as duplicates of #18460, keep #57418 closed in that chain, and exclude #22398, #26527, #28363, #28616, #29671, and #32761 from this ACP issue complex.

Complex graph

flowchart TD
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I18452(["issue #18452 (open)"])
    I64441(["issue #64441 (open)"])
    subgraph Dup18460 ["PRs duplicating each other"]
        P18460["PR #18460 (open)"]
        P22200["PR #22200 (open)"]
        P36720["PR #36720 (open)"]
        P57418["PR #57418 (closed)"]
        P64868["PR #64868 (open)"]
    end
    P64868 -->|fixes| I18452
    P64868 -->|fixes| I64441
    class I18452 open
    class I64441 open
    class P18460 open
    class P22200 open
    class P36720 open
    class P57418 closed
    class P64868 open
    class P18460 best
    class P18460 best
    class P64868 target
    click I18452 "https://github.com/NousResearch/hermes-agent/issues/18452"
    click I64441 "https://github.com/NousResearch/hermes-agent/issues/64441"
    click P18460 "https://github.com/NousResearch/hermes-agent/pull/18460"
    click P22200 "https://github.com/NousResearch/hermes-agent/pull/22200"
    click P36720 "https://github.com/NousResearch/hermes-agent/pull/36720"
    click P57418 "https://github.com/NousResearch/hermes-agent/pull/57418"
    click P64868 "https://github.com/NousResearch/hermes-agent/pull/64868"
Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 11 pull requests and 2 issues in this complex. Diffs were read for 7 of 11 PRs (rest unavailable); Assessment working set: 35 kB of PR diffs, 21 kB of issue/PR text, 12 kB of discussion (21 comments), 12 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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 P4 Best-effort: we will get to it when we get to it (no commitment) sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: ACP adapter (hermes acp) ignores fallback_providers — _make_agent never passes fallback_model, so ACP sessions run with an empty fallback chain

4 participants