refactor: remove Amazon Bedrock as first-class provider - #1212
Conversation
Bedrock's OpenAI-compatible endpoint works through the existing "compatible-endpoint" option — the same path every other CSP wrapper (Vertex, Azure OpenAI, etc.) should use. Adding a dedicated provider key for each CSP does not scale and creates maintenance overhead for config, tests, UX, and docs that is better avoided. Also adds a guard test that enumerates the approved provider set and fails if a new provider key is introduced, steering contributors toward the compatible-endpoint / compatible-anthropic-endpoint options instead. Reverts: #963 Closes: #1204
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request removes Amazon Bedrock inference provider support from the codebase. Bedrock configuration handling is removed from the provider selection function, onboarding flows, and model options. Tests specific to Bedrock are deleted, with new validation added to enforce an approved providers list. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
🚀 Docs preview ready! |
## Summary Reverts #963 and adds a guard test that prevents new CSP-specific provider keys from being added to the installer. ## Rationale Bedrock's OpenAI-compatible endpoint already works through the existing `compatible-endpoint` option — the same path every other CSP wrapper (Vertex, Azure OpenAI, etc.) should use. Adding a dedicated provider key for each CSP does not scale: each one requires config in `onboard.js`, a case in `inference-config.js`, UX menu entries, model curation, validation logic, tests, and docs. The `compatible-endpoint` and `compatible-anthropic-endpoint` options already cover these use cases generically. ## Changes - **`bin/lib/onboard.js`**: Remove `BEDROCK_ENDPOINT_URL`, `bedrock` from `REMOTE_PROVIDER_CONFIG`, `REMOTE_MODEL_OPTIONS`, `validProviders`, interactive menu, region check, validation, `setupInference`, and dashboard label. - **`bin/lib/inference-config.js`**: Remove `bedrock` case from `getProviderSelectionConfig()`. - **`test/inference-config.test.js`**: Remove Bedrock-specific assertions; add `does not grow beyond the approved provider set` guard test that fails if any new provider key (bedrock, vertex, azure, etc.) is introduced. ## Related - Reverts #963 - Closes #1204 ## Type of Change - [x] Code change for a new feature, bug fix, or refactor. ## Checklist - [x] Formatters applied - [x] Tests added (guard test) - [x] No secrets committed - [x] `npm test` passes (725 passed) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Breaking Changes** * Removed support for Amazon Bedrock as an inference provider. * **Documentation** * Updated command documentation to recommend `nemoclaw onboard` for new installations. * Marked legacy `nemoclaw setup` command as deprecated. * Updated deployment guides to reference the `nemoclaw onboard` setup wizard. * **Tests** * Updated test suite to validate approved inference providers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Deepak Jain <deepujain@users.noreply.github.com>
|
Hey @cv. I feel like the core contribution of the original push was to allow converse api (though I could be wrong). I believe Anthropic over Bedrock is only supported via Converse and InvokeModel API. (I'm also having a miserable time validating this since AWS docs are super-opaque on this. Please correct me if I'm wrong, but I think that's why bedrock is generally treated separately and may be needed.) EXECUTING
|
|
Spent a bit more time on this, since some may not have access. Can at least confirm bedrock Anthropic with chat-completions is non-trivial: Getting List of Available Modelscurl -s -X GET "https://bedrock.us-east-1.amazonaws.com/foundation-models" \ -H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \ -H "Content-Type: application/json" | jq -r '.modelSummaries[].modelId' | grep -E "anthropic|nvidia|openai" nvidia.nemotron-nano-12b-v2 anthropic.claude-sonnet-4-20250514-v1:0 anthropic.claude-haiku-4-5-20251001-v1:0 openai.gpt-oss-120b-1:0 nvidia.nemotron-nano-3-30b anthropic.claude-sonnet-4-6 nvidia.nemotron-super-3-120b anthropic.claude-opus-4-6-v1 nvidia.nemotron-nano-9b-v2 openai.gpt-oss-20b-1:0 openai.gpt-oss-safeguard-120b anthropic.claude-sonnet-4-5-20250929-v1:0 anthropic.claude-opus-4-1-20250805-v1:0 anthropic.claude-opus-4-5-20251101-v1:0 openai.gpt-oss-safeguard-20b anthropic.claude-3-sonnet-20240229-v1:0:28k anthropic.claude-3-sonnet-20240229-v1:0:200k anthropic.claude-3-sonnet-20240229-v1:0 anthropic.claude-3-haiku-20240307-v1:0:48k anthropic.claude-3-haiku-20240307-v1:0:200k anthropic.claude-3-haiku-20240307-v1:0 anthropic.claude-3-7-sonnet-20250219-v1:0 anthropic.claude-3-5-haiku-20241022-v1:0 anthropic.claude-opus-4-20250514-v1:0 Querying Nemotron Super (OK)curl -X POST "https://bedrock-runtime.us-east-1.amazonaws.com/openai/v1/chat/completions" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \
-d '{
"model": "nvidia.nemotron-super-3-120b",
"messages": [{"role": "user", "content": "Hello world" }],
"max_tokens": 10
}'
Querying GPT-OSS (OK)curl -X POST "https://bedrock-runtime.us-east-1.amazonaws.com/openai/v1/chat/completions" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \
-d '{
"model": "openai.gpt-oss-20b-1:0",
"messages": [{"role": "user", "content": "Hello world" }],
"max_tokens": 10
}'Querying Claude Opus via Chat/Completions (NOT OK)curl -X POST "https://bedrock-runtime.us-east-1.amazonaws.com/openai/v1/chat/completions" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \
-d '{"model": "anthropic.claude-opus-4-6-v1", "messages": [{"role": "user", "content": "Hello world" }], "max_tokens": 10}'
curl -X POST "https://bedrock-runtime.us-east-1.amazonaws.com/openai/v1/chat/completions" \ns" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \
-d '{"model": "global.anthropic.claude-opus-4-6-v1",
"messages": [{"role": "user", "content": "Hello world" }], "max_tokens": 10}'Querying General Claude Opus via Converse (NOT OK)curl -X POST "https://bedrock-runtime.us-east-1.amazonaws.com/model/anthropic.claude-opus-4-6-v1/converse" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \
-d '{"messages": [{"role": "user", "content": [{"text": "Hello"}]}]}'Querying Regional Claude Opus via Converse (OK)curl -X POST "https://bedrock-runtime.us-east-1.amazonaws.com/model/anthropic.claude-opus-4-6-v1/converse" \
-H "Content-Type: application/json" \
-H "Authorization: Bearer $AWS_BEARER_TOKEN_BEDROCK" \
-d '{"messages": [{"role": "user", "content": [{"text": "Hello"}]}]}'
|
## Summary Reverts NVIDIA#963 and adds a guard test that prevents new CSP-specific provider keys from being added to the installer. ## Rationale Bedrock's OpenAI-compatible endpoint already works through the existing `compatible-endpoint` option — the same path every other CSP wrapper (Vertex, Azure OpenAI, etc.) should use. Adding a dedicated provider key for each CSP does not scale: each one requires config in `onboard.js`, a case in `inference-config.js`, UX menu entries, model curation, validation logic, tests, and docs. The `compatible-endpoint` and `compatible-anthropic-endpoint` options already cover these use cases generically. ## Changes - **`bin/lib/onboard.js`**: Remove `BEDROCK_ENDPOINT_URL`, `bedrock` from `REMOTE_PROVIDER_CONFIG`, `REMOTE_MODEL_OPTIONS`, `validProviders`, interactive menu, region check, validation, `setupInference`, and dashboard label. - **`bin/lib/inference-config.js`**: Remove `bedrock` case from `getProviderSelectionConfig()`. - **`test/inference-config.test.js`**: Remove Bedrock-specific assertions; add `does not grow beyond the approved provider set` guard test that fails if any new provider key (bedrock, vertex, azure, etc.) is introduced. ## Related - Reverts NVIDIA#963 - Closes NVIDIA#1204 ## Type of Change - [x] Code change for a new feature, bug fix, or refactor. ## Checklist - [x] Formatters applied - [x] Tests added (guard test) - [x] No secrets committed - [x] `npm test` passes (725 passed) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Breaking Changes** * Removed support for Amazon Bedrock as an inference provider. * **Documentation** * Updated command documentation to recommend `nemoclaw onboard` for new installations. * Marked legacy `nemoclaw setup` command as deprecated. * Updated deployment guides to reference the `nemoclaw onboard` setup wizard. * **Tests** * Updated test suite to validate approved inference providers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Deepak Jain <deepujain@users.noreply.github.com>
## Summary Reverts NVIDIA#963 and adds a guard test that prevents new CSP-specific provider keys from being added to the installer. ## Rationale Bedrock's OpenAI-compatible endpoint already works through the existing `compatible-endpoint` option — the same path every other CSP wrapper (Vertex, Azure OpenAI, etc.) should use. Adding a dedicated provider key for each CSP does not scale: each one requires config in `onboard.js`, a case in `inference-config.js`, UX menu entries, model curation, validation logic, tests, and docs. The `compatible-endpoint` and `compatible-anthropic-endpoint` options already cover these use cases generically. ## Changes - **`bin/lib/onboard.js`**: Remove `BEDROCK_ENDPOINT_URL`, `bedrock` from `REMOTE_PROVIDER_CONFIG`, `REMOTE_MODEL_OPTIONS`, `validProviders`, interactive menu, region check, validation, `setupInference`, and dashboard label. - **`bin/lib/inference-config.js`**: Remove `bedrock` case from `getProviderSelectionConfig()`. - **`test/inference-config.test.js`**: Remove Bedrock-specific assertions; add `does not grow beyond the approved provider set` guard test that fails if any new provider key (bedrock, vertex, azure, etc.) is introduced. ## Related - Reverts NVIDIA#963 - Closes NVIDIA#1204 ## Type of Change - [x] Code change for a new feature, bug fix, or refactor. ## Checklist - [x] Formatters applied - [x] Tests added (guard test) - [x] No secrets committed - [x] `npm test` passes (725 passed) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Breaking Changes** * Removed support for Amazon Bedrock as an inference provider. * **Documentation** * Updated command documentation to recommend `nemoclaw onboard` for new installations. * Marked legacy `nemoclaw setup` command as deprecated. * Updated deployment guides to reference the `nemoclaw onboard` setup wizard. * **Tests** * Updated test suite to validate approved inference providers. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Deepak Jain <deepujain@users.noreply.github.com>
Summary
Reverts #963 and adds a guard test that prevents new CSP-specific provider keys from being added to the installer.
Rationale
Bedrock's OpenAI-compatible endpoint already works through the existing
compatible-endpointoption — the same path every other CSP wrapper (Vertex, Azure OpenAI, etc.) should use. Adding a dedicated provider key for each CSP does not scale: each one requires config inonboard.js, a case ininference-config.js, UX menu entries, model curation, validation logic, tests, and docs. Thecompatible-endpointandcompatible-anthropic-endpointoptions already cover these use cases generically.Changes
bin/lib/onboard.js: RemoveBEDROCK_ENDPOINT_URL,bedrockfromREMOTE_PROVIDER_CONFIG,REMOTE_MODEL_OPTIONS,validProviders, interactive menu, region check, validation,setupInference, and dashboard label.bin/lib/inference-config.js: Removebedrockcase fromgetProviderSelectionConfig().test/inference-config.test.js: Remove Bedrock-specific assertions; adddoes not grow beyond the approved provider setguard test that fails if any new provider key (bedrock, vertex, azure, etc.) is introduced.Related
Type of Change
Checklist
npm testpasses (725 passed)Summary by CodeRabbit
Release Notes