Conversation
…M role auth When a user switches from Bedrock to another provider (e.g. Anthropic, OpenRouter) and then opens the /model picker again, Bedrock was absent. Root cause: _has_aws_sdk_creds_for_listing() only called has_aws_credentials() when slug == current_provider. On IAM instance role auth there are no AWS_* env vars, so _has_fast_aws_sdk_signal() returns False, and then the slug != current_provider guard short-circuits to False — Bedrock never appears. Fix: drop the slug/current_provider guard. has_aws_credentials() already does a fast env-var check before touching botocore/IMDS, so calling it unconditionally is safe and covers all auth paths (env vars, ~/.aws/credentials, IAM instance role, ECS task role, etc.). Updates the stale test that asserted has_aws_credentials was never called for non-current providers; replaces it with two clearer tests — one for the no-credentials case and one for the IAM role regression. Fixes NousResearch#45838
|
Related: #45838 (the issue this closes), #45896 and #45943 (competing open fixes for the same picker-omission symptom, at different sites: This PR fixes a distinct code path — the |
teknium1
left a comment
There was a problem hiding this comment.
Thanks for isolating a real current-main omission: hermes_cli/model_switch.py:1617-1618 does hide Bedrock when an IAM-role user has switched to another provider.
Problems
- The proposed removal reverses the guard introduced by
d409a4409c8f11ccf029eff33a2eb9860f92e761(fix(model): avoid bedrock credential probe in provider picker). That guard exists because picker opens for non-Bedrock providers must not enter botocore’s full credential chain. - With no fast AWS signal, this change reaches implicit credential resolution at
agent/bedrock_adapter.py:310-319and343-350, including possible EC2 IMDS access, for ordinary unrelated picker opens. The changed test intentionally drops the existing no-probe contract attests/hermes_cli/test_bedrock_model_picker.py:219-241.
Suggested changes
- Preserve the non-Bedrock no-probe behavior and implement IAM-role re-discovery through a bounded/explicit path that does not synchronously probe AWS credentials on every picker render. Add the switch-away regression against that design.
Automated hermes-sweeper review.
| def _has_aws_sdk_creds_for_listing(slug: str) -> bool: | ||
| """Credential check for AWS SDK providers in non-runtime discovery.""" | ||
| slug_norm = str(slug or "").strip().lower() | ||
| current_norm = str(current_provider or "").strip().lower() |
There was a problem hiding this comment.
This guard was deliberately added by d409a4409 to keep non-Bedrock picker opens out of botocore's full credential chain. Removing it makes no-env picker renders reach implicit credential resolution (including potential IMDS probing); please preserve that no-probe invariant and use a bounded IAM-role discovery path instead.
Problem
Closes #45838.
When a user switches from Bedrock to another provider (Anthropic, OpenRouter, etc.) and then opens
/modelagain, Bedrock is absent from the list.Root cause:
_has_aws_sdk_creds_for_listing()inlist_authenticated_providers()has two gates:AWS_ACCESS_KEY_ID,AWS_PROFILE, etc.) — returns False on IAM instance role auth since no AWS_* vars are setslug == current_providerguard before callinghas_aws_credentials()After switching away from Bedrock,
current_provideris now e.g."anthropic". Gate 2 short-circuits ("bedrock" != "anthropic"),has_aws_credentials()is never called, and Bedrock disappears from the picker.Fix
Remove the
slug != current_providerguard.has_aws_credentials()already does a fast env-var check internally before touching botocore/IMDS — calling it unconditionally is safe and covers all auth paths: env vars,~/.aws/credentials, IAM instance roles, ECS task roles, etc.Tests
test_non_bedrock_picker_does_not_probe_full_aws_chain→ renamed and corrected to reflect new behaviour (Bedrock hidden whenhas_aws_credentials()returns False, regardless of current provider)test_bedrock_appears_after_switching_away_iam_role: regression test for the exact scenario —current_provider="anthropic", no AWS env vars, IAM role returns True → Bedrock must appearAll 22 tests in
test_bedrock_model_picker.pypass.