Skip to content

fix(agent): initialize Bedrock clients on model switch - #67970

Open
shaoohh wants to merge 1 commit into
NousResearch:mainfrom
shaoohh:agent/fix-bedrock-switch-initialization
Open

fix(agent): initialize Bedrock clients on model switch#67970
shaoohh wants to merge 1 commit into
NousResearch:mainfrom
shaoohh:agent/fix-bedrock-switch-initialization

Conversation

@shaoohh

@shaoohh shaoohh commented Jul 20, 2026

Copy link
Copy Markdown

Summary

Fixes #41296 for the remaining initialization gap when switching to AWS Bedrock during a live session.

  • route Bedrock Claude switches to AnthropicBedrock with the region parsed from the Bedrock runtime URL
  • route Bedrock Converse switches to the SDK-only path and reload configured guardrails
  • restore Bedrock region and guardrail state if the client rebuild fails

Validation

  • git diff --check
  • parsed the changed Python modules with ast.parse
  • verified the Bedrock runtime URL regex against an ap-northeast-1 endpoint
  • attempted the repository-prescribed scripts/run_tests.sh target; this Windows environment has no installed WSL distribution, and its Python lacks the project test dependencies (pytest, PyYAML), so the targeted pytest suite could not run locally.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/bedrock AWS Bedrock (boto3, IAM) P2 Medium — degraded but workaround exists labels Jul 20, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related to #41296 and the closed-unmerged predecessors #65433 and #67948. This is the active refile; the live patch restores regional Bedrock initialization and rollback state rather than duplicating an open implementation.

@shaoohh
shaoohh marked this pull request as ready for review July 20, 2026 14:17

@teknium1 teknium1 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.

Thanks for preserving the Bedrock initialization and rollback state. The underlying runtime gap is still present on current main, but a current pipeline change prevents the Claude half of this patch from being used by normal /model switches.

Problems

  • hermes_cli/runtime_provider.py:2120-2133 resolves Claude-on-Bedrock as anthropic_messages, but hermes_cli/model_switch.py:1475-1477 then unconditionally applies the Bedrock host mode from hermes_cli/providers.py:601-602, changing it to bedrock_converse. Consequently, the new AnthropicBedrock branch is not reached for a standard Claude /model switch.
  • The added tests pass api_mode directly to agent.switch_model(), so they do not exercise that resolution-and-override path.

Suggested changes

  • Preserve the resolved Claude Bedrock anthropic_messages mode through the model-switch pipeline, while retaining Converse for non-Claude targets.
  • Add an end-to-end regression covering both Claude and non-Claude Bedrock switches through hermes_cli.model_switch.switch_model() and the live-agent runtime switch.

Automated hermes-sweeper review.

agent._client_kwargs = {}
agent.client = MoAClient(agent.model or "default")
elif (new_provider or "").strip().lower() == "bedrock":
# Bedrock does not expose an OpenAI-compatible endpoint. Mirror

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.

This branch is correct only if api_mode reaches it as anthropic_messages. On current main, the normal /model pipeline resolves Claude Bedrock as Messages in hermes_cli/runtime_provider.py:2120-2133 but then overwrites it to bedrock_converse in hermes_cli/model_switch.py:1475-1477. Please preserve the resolved Claude mode and add a pipeline-level regression; the direct helper test alone cannot catch this.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 30, 2026
@GottZ

GottZ commented Aug 3, 2026

Copy link
Copy Markdown

This was generated by AI during triage.

Summary

Twenty-five PRs address or reference this issue complex across context-length ownership and persistence, Desktop assignment, /insights attribution, Bedrock switching, and Codex replay state. The diffs range from clearing or restoring model.context_length during runtime changes to fixing separate model-switch surfaces and state, so consolidation should preserve the issue-specific canonical branches rather than combine them indiscriminately.

Related pull requests

Duplicates

#21509 is the narrower duplicate of #22387, whose implementation was merged through #24724; #17245 is the narrow precursor to the broader #17269 approach; #59191 is a closed duplicate of canonical #59125; and #41322#65433#67948#67970 is the Bedrock salvage/refile chain, with #67948 directly superseded by #67970.

Suggested consolidation

Author action: rebase #67970 onto main, preserve Claude Bedrock's resolved anthropic_messages mode through hermes_cli.model_switch, and add pipeline-level Claude and non-Claude regressions, or split that pipeline correction from the already useful regional initialization and rollback changes. Keep the issue-specific canonical branches #10366, #41270/#70210, #41965, and #59125 on their documented salvage paths; the duplicate chains are already closed except for active canonical endpoints, so no additional duplicate closure is supported by the supplied states.

Complex graph

flowchart LR
    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
    I41296(["issue #41296 (open)"])
    subgraph Dup41322 ["PRs duplicating each other"]
        P41322["PR #41322 (closed)"]
        P65433["PR #65433 (closed)"]
        P67948["PR #67948 (closed)"]
        P67970["PR #67970 (open)"]
    end
    P67970 -->|best fix| I41296
    class I41296 open
    class P41322 closed
    class P65433 closed
    class P67948 closed
    class P67970 open
    class P41322 best
    class P65433 best
    class P67970 best
    class P67970 target
    click I41296 "https://github.com/NousResearch/hermes-agent/issues/41296"
    click P41322 "https://github.com/NousResearch/hermes-agent/pull/41322"
    click P65433 "https://github.com/NousResearch/hermes-agent/pull/65433"
    click P67948 "https://github.com/NousResearch/hermes-agent/pull/67948"
    click P67970 "https://github.com/NousResearch/hermes-agent/pull/67970"
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 25 pull requests and 7 issues in this complex. Each diff was read against this issue; Assessment working set: 136 kB of PR diffs, 70 kB of issue/PR text, 32 kB of discussion (47 comments), 54 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

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/bedrock AWS Bedrock (boto3, IAM) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

4 participants