feat(workflow-executor): support Amazon Bedrock as a self-hosted AI provider - #1897
Conversation
3 new issues
|
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (9)
🛟 Help
|
|
Addressed the two bot findings in d1d3855:
Coverage stayed at 100% on all modified files. On the red checks, none is this PR:
Same SHA was green on Sept 11; the Trivy and flake failures both appeared on a re-run of unchanged code. |
Scra3
left a comment
Scra3
left a comment
There was a problem hiding this comment.
Claude Opus 5 (claude-opus-5[1m]): Spec (no ticket) — no Linear ticket governs this PR. Searched the linear-code[bot] linkback, the body, the branch name, the title and the commit trailers, then the workspace itself: bedrock returns zero issues. The two ancestors that established the AI_PROVIDER path (PRD-63 anthropic, PRD-134 openai) are Done and do not govern this one. The functional check was therefore limited to the PR description.
Claude Opus 5 (claude-opus-5[1m]): Should fix
Applies to: the PR as a whole.
A reader six months from now cannot tell whether the provider asymmetry was reasoned or accidental, and the natural instinct on finding it will be to "fix" the inconsistency.
This change makes two decisions that are hard to reverse, surprising without context, and carry a real trade-off — the /adr gate, all three met:
- Bedrock is gated by an allowlist (
supported-models.ts) whileopenaiandanthropicuse denylists. The inversion is deliberate — we vouch for nothing on a catalogue we do not own — but nothing records that. - Bedrock is executor-only:
Routerrefuses it at construction, so the sameAiConfigurationunion is served by one surface and refused by the other.
Both were reached independently by the ADR lens and the architecture lens this run, and no accepted ADR covers either (13 incidental matches, healthy control query). Worth /adr.
5e00dc8 to
1119a43
Compare
Verification of the review's body finding — run
|
…rovider `AI_PROVIDER` accepted `openai` and `anthropic` only, so customers who require their LLM calls to stay inside their AWS account had no path. Bedrock speaks the Converse API behind SigV4, which neither ChatOpenAI nor ChatAnthropic can reach. Scope is the self-hosted executor: AiClient / createBaseChatModel, the CLI env config and the embedded `ai` option. The front-facing Router keeps rejecting bedrock, pinned by a test — nothing feeds it a bedrock configuration. Credentials come from the standard AWS chain (IAM role, AWS_ACCESS_KEY_ID / AWS_SECRET_ACCESS_KEY, shared profile), so `AI_API_KEY` is refused at boot instead of being silently ignored, and a missing region is refused too rather than failing on the first AI step of the first run. Two LangChain behaviours needed working around: - `@langchain/aws` infers tool_choice support from a hardcoded family list (claude-3/4, mistral-large) and throws client-side for everything else. The executor binds every AI step with `tool_choice: 'any'`, so Claude 5 and Nova would have been dead on arrival. We pass supportsToolChoiceValues explicitly and let Bedrock be the authority; a model that genuinely cannot do it answers with a ValidationException and its id goes to the denylist. - It falls back to AWS_DEFAULT_REGION only, while ECS/EKS/Lambda set AWS_REGION. Bedrock ids wrap the vendor's own id in an inference-profile prefix and a version suffix (`us.anthropic.claude-opus-4-20250514-v1:0`), so supported-models.ts normalises them before testing — otherwise the Anthropic denylist stops matching and models we already know are broken sail through. `@langchain/aws` is pinned to 1.4.0: 1.4.5 requires @langchain/core ^1.2.9 and the repo pins 1.1.48. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The agent package lints `src test`, not just `src`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… chain A customer's devops asked whether they can assume an IAM role instead of handing us keys. They can, and it needs no code — but only for as long as we keep passing no `credentials` to ChatBedrockConverse. Wiring static keys in there later would silently disable profile `role_arn` and IRSA on EKS, and nothing would have caught it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The embedded path accepted `ai: { provider: 'bedrock' }` with no region
anywhere, reported the executor as started, and only failed on the first AI
step of the first run — while the standalone CLI refuses at boot. The Bedrock
client resolves its region from AWS_DEFAULT_REGION only and never reads the AWS
profile, so an unset region can never recover on its own.
Also extracts parseBedrockConfig so parseAiConfig stops growing a branch per
provider.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1.4.0 was the oldest version accepting @langchain/core ^1.0.0, not the newest: 1.4.1 through 1.4.3 take the same peer range. 1.4.4+ require core ^1.2.8 and the repo pins 1.1.48, so 1.4.3 is the ceiling. 1.4.3 normalises and replays Bedrock reasoning blocks with their signatures — the Bedrock-side counterpart of the dropped-thinking-block problem that already denylists claude-fable-5 on the Anthropic path, and the executor replays tool results on every AI step. supportedToolChoiceValuesForModel is unchanged in 1.4.3 (still only claude-3/4 and mistral-large), so the explicit supportsToolChoiceValues override stays necessary; re-checked that Claude 4.6, Claude 5 and Nova all still resolve tool_choice 'any'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The gate was a denylist, so everything Bedrock sells passed by default. That was indefensible: amazon.titan-text-express-v1 sailed through despite having no tool use at all, while every AI step forces a tool call. The permissive default turned "we never checked this model" into a mid-run failure for the customer. Now an allowlist. Only anthropic.claude-sonnet-*, claude-haiku-* and claude-opus-* are accepted, and they still go through the Anthropic rules after unwrapping, so the streaming-only and EOL Claude releases stay rejected. Nova, Llama, Mistral, Titan and the Claude lines outside those three are refused at startup. Drops BEDROCK_UNSUPPORTED_MODELS: an allowlist supersedes it, and leaving an empty denylist behind would suggest a second place to maintain. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven findings, five of them verified by running the code rather than reading it. Denylist bypassed by real Bedrock ids. BEDROCK_VERSION_SUFFIX was anchored to `-v1:0`, but Bedrock publishes context-window variants (`-v1:0:200k`). The id was never unwrapped, never matched ANTHROPIC_UNSUPPORTED_MODELS, and the two releases banned for requiring streaming sailed through — a hang at runtime, not an error, and invisible to the integration test, which filters on ON_DEMAND. AIModelNotSupportedError lied to Bedrock users. An allowlist refusal reported "does not support tools" for Claude 3.5 Sonnet, which supports them, sending the operator to find a capability they already had. Allowlist refusals now raise AIModelNotAllowlistedError, which names the supported lines and says ARNs are not accepted. Embedded agent swallowed an apiKey on bedrock while the CLI refuses it. The guard read `ai.provider !== 'bedrock' && !ai.apiKey`, so the bedrock-with-a-key case was never examined. ChatBedrockConverse drops the field and falls back to the ambient IAM role: wrong account, wrong bill, no error, and the operator debugs AWS while holding a key the code discarded. Router approved bedrock, then the dispatcher refused it with a 400 on every end-user query — after the type check, the startup validation and the provider metadata had all said yes. It is now refused at construction, naming the executor as the path that does support it. The integration test could not report. Its unknown-model case threw synchronously outside expect().rejects (the allowlist intercepts before invoke), so it would have failed on its first ever run; and the matrix filed failures by matching message text, so one broken InvokeModel policy filed the whole catalogue under "unavailable" and passed green. Now keyed on the AWS error name, with a floor assertion — a suite that verified nothing is broken, not passing. CLI --help still advertised 'anthropic' | 'openai' and never mentioned AWS_REGION, in the same file as the change. Adds bedrock-tool-choice.test.ts: the supportsToolChoiceValues override is the reason this provider works, and until now it was only ever asserted against a jest.fn(). The contract is verifiable offline, negative control included. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two High findings from the PR review, both cases where the allowlist refused a model Bedrock serves happily. Anthropic renamed its lines mid-Claude-4: `claude-3-5-sonnet` became `claude-sonnet-4-6`. Matching only the new spelling excluded every Claude 3.x id, so `anthropic.claude-3-5-sonnet-20241022-v2:0` was refused at startup with a message telling the operator to find a model supporting function calling — which that one does. Both spellings are live on Bedrock. An inference-profile ARN is a documented modelId and its tail is the profile id, so the ARN form reduces to the id form and is now resolved. Application inference profiles keep an opaque id that cannot be resolved without calling Bedrock, so they stay refused — and that is the form AWS documents for per-team cost allocation, so it may need revisiting. Also adds '-live' to the OpenAI unsupported patterns. gpt-live-1 is not a chat model and has been failing llm.integration on main for days. Folded in here rather than left alone because the Bedrock suite now runs in that same job: a permanently red job would hide a Bedrock regression. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… we advertise Skipping is right on a laptop and a lie in CI. Today a missing BEDROCK_AWS_REGION secret turns "nobody configured this" into a green run that reads as "Bedrock is verified" — the job passed its last run with `5 skipped` and told us nothing. The suite now throws in CI when no region is reachable, naming the three secrets. Adds a suite over the models we actually advertise, separate from the catalogue sweep. The sweep only covers what the CI account happens to enable, so it can go green having verified two models; this list does not move. An AccessDenied on one of these is a failure rather than an excuse: it means we cannot invoke a model we tell customers to use. Claude 5 leads the list on purpose. It is the line LangChain's own inference rejects, so it is the reason supportsToolChoiceValues is overridden at all, and a regression there would otherwise be invisible. The geo prefix is derived from the region rather than pinned: an `eu.` profile id is invalid in us-east-1 and would have failed the suite for a reason unrelated to the code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A customer setting AI_MODEL to a Claude line their region does not serve, or one their account has not enabled, passes every check we have: parseBedrockConfig validates the region, the allowlist recognises a legitimate Claude id, and the executor boots healthy. Nothing here can know where AWS deploys a model without calling AWS, which config validation does not do. So it fails on the first AI step, and the workflow author saw "An unexpected error occurred" — the raw provider error is not a WorkflowExecutorError, so it never even reached the AI-service wording. That points them at the workflow when the fix is in AI_MODEL or AWS_REGION. ValidationException, ResourceNotFoundException and AccessDeniedException are all operator-fixable configuration mistakes, so they now carry the provider's own sentence plus the model id. These AWS messages name the model and the reason and never a credential, which is what makes passing them through safe. Everything else still propagates untouched: a network failure is not a configuration error and must not be dressed up as one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The advertised-models suite pinned claude-sonnet-5, claude-sonnet-4-6, claude-haiku-4-5 and claude-opus-4-5. Three of those were a guess: the eu-west-1 catalogue carries Opus 5, Sonnet 5, Opus 4.8 and Opus 4.7, so the suite would have failed on ids AWS does not serve there — a red run about my assumption rather than about the code. Which releases a region carries is AWS's call and moves without us. What must hold is narrower and does not rot: every line we advertise (sonnet, haiku, opus) has at least one model this account can actually drive with a forced tool call. The candidates come from the same discovery the sweep uses, so the assertion follows the region instead of fighting it. The smoke tests stop depending on a pinned id too, and the discovery moves to the root describe so both suites share one round of ListFoundationModels plus ListInferenceProfiles. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…-of-life First real run of the integration suite, against eu-west-1 with live credentials. Of the 21 models the allowlist admits there, 20 answer a forced tool call and one does not: eu.anthropic.claude-3-sonnet-20240229-v1:0 "This model version has reached the end of its life." Exactly what the suite exists to catch, and not something any amount of reading would have found: the model is listed, on-demand, and looks ordinary. An earlier commit in this PR asserted the opposite — that this id was allowed — on the strength of it being a legitimate claude-3 sonnet. AWS disagreed. Two test fixes that the same run forced: The per-line check took candidates[0], which is the oldest release in a sorted list and therefore the one closest to retirement — it failed on the legacy model while newer ones worked. It now tries candidates until one answers, since "at least one model per line" is the actual claim. That loop first swallowed the reason with .catch(() => false), which hid two real findings behind a bare "could not be driven": a missing Marketplace entitlement and a policy too narrow for global. profiles. It now reports each candidate's error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g on the first model `yarn test:openai`, `yarn test:anthropic` and `yarn test:bedrock`. OpenAI and Anthropic share a file and were selected by which key happened to be exported, so running one on purpose meant remembering a -t filter. The two Bedrock smoke tests still took modelsToTest[0]. Sorted, that is the oldest release — the one most likely to be retired or left unentitled — so they failed on an account problem while reporting as if Bedrock had rejected our request shape. They now walk the list until a model answers, like the per-line checks already do, and report every candidate's error when none does. The sweep's floor assertion no longer pins a specific id either. Against eu-west-1: 6 of 8 pass. The two failures are real and not code — Haiku 4.5 and Opus 5 have no Marketplace entitlement on this account, which is exactly what the per-line checks exist to say out loud. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The example package had a one-command launcher per provider, openai and anthropic, and nothing for bedrock — so trying the new provider locally meant reconstructing the concurrently invocation by hand. It cannot be a copy of the other two: bedrock takes no AI_API_KEY, needs AWS_REGION, and `source .env` would happily leak an AI_API_KEY into a bedrock run, which the executor rejects at boot. The script unsets it and defaults the region, with BEDROCK_MODEL overridable. Documents the three launchers together in the README, where only the plain start:with-executor was mentioned. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The comment above the env block still said the suite skips itself when AWS_REGION is unset. This PR changed that on purpose — a silent skip would let the provider rot untested — so the comment now says what the job does and which secrets make it pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…om leaking the account Two defects the validator's silent-failure lens found in the error this PR added one commit earlier, both of which defeat its stated purpose. The error never reached the author on the two step types most likely to hit it. withAiAssist re-tags every error as AiAssistUnavailableError, and guidance and load-related-record call the model through it — so a model the region does not serve degraded to a manual step, forever, with a Warn log as the only trace. The other six executors surfaced it correctly, which made the same misconfiguration loud on some steps and silent on others. AiModelUnusableError now passes through: a refused model is a permanent configuration fault, not the transient outage that path exists for. userMessage carried the provider's sentence into the Forest UI and the audit trail, where an AccessDeniedException spells out the caller's IAM role ARN and AWS account id for every reader of the run — visible in this session's own test output. The comment claiming those messages "never name a credential" was right about credentials and wrong about identity. The provider sentence moves to the technical message; userMessage names the model and the error class. It also shipped unclassified: extending WorkflowConfigurationError stamps the 'configuration' errorKind the class's own comment describes. And the remedy sentence no longer asserts a cause the classifier cannot establish — ValidationException is Bedrock's catch-all for request shape too, so "check the model id and region" sent the operator after a correct configuration when the real cause was an oversized prompt. Three tests added, including the withAiAssist interaction that no test covered, which is why this went unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The package lints `src test`; I had only linted `src`, so an unsorted import and a prettier break reached CI. Same miss as on the agent package earlier in this branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The matrix filed AccessDenied and Throttling in one bucket and excluded both from failures, so a run where two models answered and thirty-eight were throttled reported the same green as a fully verified one — with the ratio only in a console.log nobody opens when the job is green. The suite also manufactured the throttling it excused: it built clients through AiClient directly, inheriting createBaseChatModel's maxRetries 0, where production goes through AiClientAdapter with 2. Throttling is now its own bucket and an empty-throttled assertion: it is our own serial walk with no backoff, so it means the run is broken, not that a model failed. Entitlement stays excluded — that one is a fact about the account. A third account state turned up on the first real run and is excluded too: AWS parks a Legacy model an account has not called in 30 days, which answers fine on an account that uses it. Read from the message, not the exception name, because it shares ResourceNotFoundException with a genuine end-of-life — and that one has to stay a failure, since it is how a retired id earns its denylist entry. Verified against a real AWS account in eu-west-1: 8/8, all three advertised Claude lines driven with a forced tool call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two choices in this PR read as inconsistencies and are not: Bedrock is gated by an allowlist where the other providers use denylists, and it is accepted by the executor while the server-side proxy refuses it at construction. The decisive reason is not in the code and could not be inferred from it — the server-side provider choice is being retired, so wiring Bedrock into the dispatcher would invest in a surface we are removing. Without that written down, the natural instinct on finding the asymmetry is to harmonise it, which removes a deliberate protection. Raised independently by the ADR and architecture lenses of the validator run on this PR, and no existing ADR covered it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…hbours The comment credited the end-of-life to Bedrock's error message, which is where it was observed but not where it was decided. Anthropic retired claude-3-sonnet-20240229 on 2025-07-21 on its own API, so the entry belongs in this list on the same grounds as every other dated one, and the date is the fact worth carrying. It also settles what the entry costs: no customer can hold a working configuration on a retired model, so the executor now refuses at boot what it used to accept and then fail on at every AI step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n as a model fault ValidationException is Bedrock's catch-all: an oversized prompt raises it just like a bad model id does. Treating it as configuration made AiModelUnusableError bypass the withAiAssist degrade path, so a large record failed the step permanently on a model that works. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
e879b44 to
032024e
Compare
EnkiP
left a comment
EnkiP
left a comment
There was a problem hiding this comment.
Spec (no ticket): the functional check was limited to the PR description. It conforms except one line: the description says a ValidationException surfaces as AiModelUnusableError, and since 032024e it degrades or lands in the generic error instead.
| // The CLI and the embedded option both refuse this, but a direct `new AiClient(...)` reaches | ||
| // neither, and ChatBedrockConverse drops the key silently and authenticates with the ambient | ||
| // role instead — wrong account, wrong bill, no error. | ||
| if (config.provider === 'bedrock' && (config as { apiKey?: string }).apiKey) { |
There was a problem hiding this comment.
Claude Fable 5.1 (claude-fable-5-1): Violates conventions: the AiClient guard refusing an apiKey on a bedrock configuration has no test, so a refactor dropping it passes CI.
Code
validate-ai-configurations.ts:11 throws for a bedrock config carrying apiKey, no test reaches it, and testing.md#Cover error and edge paths asks for one per branch.
A way out: one it() beside ai-client.test.ts:54 passing apiKey on a bedrock configuration.
There was a problem hiding this comment.
Fixed in 25fe21f — correct, the guard had no test reaching it.
Added one beside the allowlist cases in ai-client.test.ts, asserting the thrown message. Worth stating why this guard earns a test rather than being deleted as redundant with the two entry points: a direct new AiClient(...) meets neither, and the failure is silent — ChatBedrockConverse drops the key and authenticates with the ambient role instead, so the call succeeds against an account nobody chose and bills it. A refactor dropping the guard would have shipped green.
| // LangChain reads AWS_DEFAULT_REGION only, and its own error names just that one — unhelpful to | ||
| // anyone who set AWS_REGION, the variable the AWS SDK and CLI treat as primary. | ||
| function resolveBedrockRegion(region?: string): string { | ||
| const resolved = region ?? process.env.AWS_REGION ?? process.env.AWS_DEFAULT_REGION; |
There was a problem hiding this comment.
Claude Fable 5.1 (claude-fable-5-1): Should fix: an embedded agent with an empty AWS_REGION and a set AWS_DEFAULT_REGION boots, then every AI step fails as unavailable.
Code
create-base-chat-model.ts:19 resolves with ?? so the empty string wins, while embedded-workflow-executor.ts:97 treats it as unset and the lazy ai-client.ts:36 build throws inside a step.
A way out: || in resolveBedrockRegion, matching cli-core.ts:181.
Reproduction
- Set
AWS_REGION=empty andAWS_DEFAULT_REGION=eu-west-3, then embed the executor with a bedrockaioption and noregion. - Call
agent.start()and observe it succeeds. - Run a workflow with an AI step and observe it fails with the AI service unavailable message.
There was a problem hiding this comment.
Fixed in 25fe21f — real, and the reproduction is exact.
resolveBedrockRegion used ?? where cli-core.ts:181 uses ||, so the two entry points disagreed on what an empty string means. The embedded boot check reads '' as unset and lets the deployment through; ?? then keeps it and throws at model construction, inside the first AI step. Reproduced before the fix:
AWS_REGION='' AWS_DEFAULT_REGION=eu-west-3
→ THROW: AIBadRequestError - Bedrock requires a region…
Now ||, matching the standalone path, with a test asserting the fallback to AWS_DEFAULT_REGION when AWS_REGION is set but empty. An exported-but-unfilled variable is how a container ships a value it never got, so it has to read as absent rather than as a refusal.
| // the no-degrade path below and fail the step permanently instead of falling back to manual. The | ||
| // cost is that a typo in AI_MODEL (also a ValidationException) degrades to manual rather than | ||
| // naming itself; the warn log carries Bedrock's sentence. | ||
| const AI_CONFIGURATION_ERROR_NAMES = ['ResourceNotFoundException', 'AccessDeniedException']; |
There was a problem hiding this comment.
Claude Fable 5.1 (claude-fable-5-1): Should fix: a mistyped Bedrock model id that passes the allowlist fails condition, record and MCP steps with an unclassified unexpected error.
Code
errors.ts:322 omits ValidationException, which Bedrock returns for a bad id that supported-models.ts:164 let through, so base-step-executor.ts:94 logs an unexpected error with no errorKind.
A way out: classify ValidationException on the model identity sentence only, model identifier is invalid, keeping oversized prompts on the degrade path.
Reproduction
- Start the executor with
AI_PROVIDER=bedrock,AI_MODEL=eu.anthropic.claude-sonnet-4-7-v1:0andAWS_REGION=eu-west-1, which boots. - Run a workflow whose first step is a condition step.
- The step fails with
Unexpected error during step executionand no configuration classification.
There was a problem hiding this comment.
Fixed in 25fe21f — right, and this one caught a regression I introduced earlier today.
Sequence: Macroscope raised the opposite defect on the same line (ValidationException classified as configuration sends an oversized prompt down the no-degrade path). I removed the name outright, which fixed that and broke this one. Both findings are correct about their own case, and the middle ground you propose is the only thing that satisfies both — I had considered it and rejected it for a bad reason: I could not verify AWS's wording from here, so I preferred asserting nothing.
That reasoning was wrong, because the failure modes are not symmetric. A pattern that fails to match costs one classification and leaves today's behaviour; a pattern that matches too much stops a workflow a human could have finished. So a narrow match with "not configuration" as the default is strictly better than no match.
Implemented as /model identifier/i, checked only for ValidationException. It cannot match Input is too long for requested model, which is the case that has to keep degrading.
The part I could not do from a laptop, I moved to CI instead of guessing: bedrock.integration.test.ts now invokes a well-formed but nonexistent id against the real account and asserts the failure is classifiable — by exception name, or by that sentence. If AWS rewords it, CI turns red and prints the actual string, which is how we learn before a customer does.
On the premise itself: confirmed that condition-step-executor.ts:281 and mcp-step-executor.ts:318 call invokeWithTool directly, outside withAiAssist, so an unclassified error there does surface as "Unexpected error during step execution" with no errorKind.
…model-identity error Three findings from the validator run on this PR: - resolveBedrockRegion used ?? where cli-core uses ||, so a container exporting an empty AWS_REGION passed the boot check and failed on the first AI step. - The AiClient guard refusing an apiKey on a bedrock config had no test. - Narrowing isAiConfigurationError to drop ValidationException left a mistyped AI_MODEL unclassified on condition and MCP steps, which do not go through the degrade path. Classify it again, but only on the model-identity sentence, and pin that sentence with an integration test so a reword turns CI red. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| fail-on-cache-miss: true | ||
| - name: Run LLM integration tests | ||
| run: yarn workspace @forestadmin/ai-proxy test --testPathPattern='llm.integration' | ||
| run: yarn workspace @forestadmin/ai-proxy test --testPathPattern='llm.integration|bedrock.integration' |
There was a problem hiding this comment.
🟡 Medium workflows/build.yml:150
The bedrock.integration suite cannot block publishing or Pages deployment: failures in llm-integration-tests are tolerated by continue-on-error: true, and neither downstream job depends on it. Move Bedrock into a required job (or otherwise make its result a required dependency) while keeping the non-gating LLM tests separate.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/build.yml around line 150:
The `bedrock.integration` suite cannot block publishing or Pages deployment: failures in `llm-integration-tests` are tolerated by `continue-on-error: true`, and neither downstream job depends on it. Move Bedrock into a required job (or otherwise make its result a required dependency) while keeping the non-gating LLM tests separate.
There was a problem hiding this comment.
Correct on the facts, and it caught an overclaim of mine — corrected in 037198e.
continue-on-error: true on llm-integration-tests and the absence of any downstream needs are both pre-existing on main; this PR only extended the test pattern and added the Bedrock env. But my own comment in bedrock.integration.test.ts claimed the CI guard turns a missing region into "a failure", which under continue-on-error is a red job nobody is gated on. That comment now says what is true.
I am not moving Bedrock into a gating job, and the reason is the one that put continue-on-error there in the first place: every suite in that job reaches a third party. The Bedrock sweep in particular walks the catalogue serially with maxRetries: 0 and already fails the run on throttling — deliberately, so a mostly-throttled sweep cannot read as proof. Gating on it would redden main whenever AWS throttles or an entitlement lapses, which is a worse failure than the one being prevented.
Splitting the deterministic part (secrets present) from the flaky part (the sweep) into two jobs would give the gate without the flakiness, and I would support it — but as a change to how all three providers gate, not as a Bedrock-only exception landed here. Happy to open a ticket for it.
There was a problem hiding this comment.
Thanks for clarifying. Given the established non-gating policy for third-party suites, I agree Bedrock should not be made a one-off gate here. Would you like me to prepare the broader split of deterministic credential checks from provider sweeps in a separate PR against main?
| # The Bedrock suite fails the job when AWS_REGION is unset: a silent skip would let the | ||
| # provider rot untested. Set BEDROCK_AWS_* in the repository secrets. | ||
| AWS_REGION: ${{ secrets.BEDROCK_AWS_REGION }} | ||
| AWS_ACCESS_KEY_ID: ${{ secrets.BEDROCK_AWS_ACCESS_KEY_ID }} |
There was a problem hiding this comment.
🔴 Critical workflows/build.yml:157
This pull_request workflow exposes long-lived AWS credentials to PR-controlled ai-proxy test code, allowing same-repository contributors to exfiltrate them or use them against AWS. Run this suite only on protected push/scheduled/manual contexts, or replace these credentials with short-lived OIDC access behind a protected environment.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @.github/workflows/build.yml around line 157:
This `pull_request` workflow exposes long-lived AWS credentials to PR-controlled `ai-proxy` test code, allowing same-repository contributors to exfiltrate them or use them against AWS. Run this suite only on protected push/scheduled/manual contexts, or replace these credentials with short-lived OIDC access behind a protected environment.
There was a problem hiding this comment.
Real, and worth a ticket — but it is not specific to this PR and I am not fixing it here.
The exposure is pre-existing: OPENAI_API_KEY and ANTHROPIC_API_KEY sit in the same env block on main, in the same pull_request-triggered job. This PR adds a third secret to an established pattern. Fixing it properly — OIDC with a short-lived role behind a protected environment — covers all three and changes how the repo handles CI credentials; doing it for AWS alone inside a Bedrock PR would leave the other two exposed and bury a repo-wide decision in an unrelated review.
Two things that bound the blast radius, neither of which makes the finding wrong:
- The trigger is
pull_request, notpull_request_target, so fork PRs get no secrets at all. The reachable attacker is someone who already has write access to this repository. - The IAM user is scoped to Bedrock, and narrowly: this run's own log shows it denied on
bedrock:InvokeModelfor theglobal.*profiles. It is not a general-purpose AWS credential.
Separately, and regardless of this finding: that key needs rotating for an unrelated reason — it was shared in clear while we were setting the CI up, before a dedicated one existed. That was already on the list.
There was a problem hiding this comment.
Agreed that the underlying CI-secret exposure should be addressed repo-wide. Would you like me to prepare a separate PR against main?
llm-integration-tests carries continue-on-error and no job depends on it, so a missing-secrets failure is a red job nobody is gated on — not the gate the comment implied. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main already carried it; the second entry was dead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
# @forestadmin/ai-proxy [1.14.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/ai-proxy@1.13.1...@forestadmin/ai-proxy@1.14.0) (2026-09-15) ### Features * **workflow-executor:** support Amazon Bedrock as a self-hosted AI provider ([#1897](#1897)) ([53dfe75](53dfe75))
# @forestadmin/workflow-executor [1.29.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/workflow-executor@1.28.5...@forestadmin/workflow-executor@1.29.0) (2026-09-15) ### Features * **workflow-executor:** support Amazon Bedrock as a self-hosted AI provider ([#1897](#1897)) ([53dfe75](53dfe75)) ### Dependencies * **@forestadmin/ai-proxy:** upgraded to 1.14.0
# @forestadmin/agent [1.102.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/agent@1.101.0...@forestadmin/agent@1.102.0) (2026-09-15) ### Features * **workflow-executor:** support Amazon Bedrock as a self-hosted AI provider ([#1897](#1897)) ([53dfe75](53dfe75)) ### Dependencies * **@forestadmin/workflow-executor:** upgraded to 1.29.0
3 participants
What
AI_PROVIDERacceptedopenaiandanthropiconly. Customers who need their LLM calls to stay inside their AWS account had no path — Bedrock speaks the Converse API behind SigV4, which neitherChatOpenAInorChatAnthropiccan reach.AI_PROVIDER=bedrock AI_MODEL=eu.anthropic.claude-sonnet-5 AWS_REGION=eu-west-1 # no AI_API_KEYVerified against a real AWS account in
eu-west-1: the integration suite passes 8/8, with all three advertised Claude lines driven through a forced tool call.Scope, and why the proxy still refuses it
Self-hosted executor only:
AiClient/createBaseChatModel, the CLI env config, and the embeddedaioption ofaddWorkflowExecutor().The front-facing
Router/ProviderDispatcherkeeps refusingbedrock, and now does so at construction rather than on the first end-user query. The reason is not that nothing feeds it: the server-side AI provider choice is being retired — the SaaS will serve OpenAI only, and customisation will go through the embedded executor — so wiring Bedrock into the dispatcher would build on a surface we are removing. Recorded indocs/adr/2026-09-15-bedrock-is-executor-only-and-its-models-pass-an-allowlist.md, because the asymmetry reads as an inconsistency and the natural instinct is to "fix" it.Supported models: an allowlist, not a denylist
Claude sonnet, haiku and opus only — as a bare id, behind any cross-region inference-profile prefix (
us.,eu.,apac.,jp.,au.,global.), or as an inference-profile ARN, which is reduced to its id. Application inference profiles carry an opaque id and are refused.This inverts what
openaiandanthropicuse, on purpose: Bedrock resells hundreds of models we do not own, and the executor forces a tool call on every AI step.amazon.titan-text-express-v1passed a permissive filter without being able to honour one, which surfaces as a failure mid-run instead of a refusal at startup.Ids are unwrapped before matching, then still run through the Anthropic rules — that is what keeps the streaming-only and retired Claude releases refused here too, including through Bedrock's context-window suffixes (
-v1:0:200k).Credentials
The standard AWS chain (IAM role,
AWS_ACCESS_KEY_ID/AWS_SECRET_ACCESS_KEY, shared profile, assumed roles and IRSA). Two consequences, both checked at boot rather than mid-workflow:AI_API_KEYalongsidebedrockis refused, not silently ignored —ChatBedrockConversedrops it and falls back to the ambient role, so an operator would ship a key believing it authenticates.AWS_REGION/AWS_DEFAULT_REGIONis refused.ChatBedrockConversedemands a region at construction and readsAWS_DEFAULT_REGIONonly, so the AWS SDK below it never gets to resolve one from the shared profile.Two LangChain behaviours worked around
1.
tool_choiceis gated by a stale hardcoded list.@langchain/awsinfers support from model-family substrings (claude-3,claude-4,mistral-large) and throws client-side for anything else. The executor binds every AI step withtool_choice: 'any', so this is on the critical path. Measured against the unpatched default:We pass
supportsToolChoiceValuesexplicitly and let Bedrock be the authority: a model that genuinely cannot do it answers with aValidationException, and its id leaves the allowlist.2. Region. LangChain reads
AWS_DEFAULT_REGIONonly, while ECS/EKS/Lambda setAWS_REGION.Errors an operator can act on
A model the provider refuses (
ValidationException,ResourceNotFoundException,AccessDeniedException) used to reach the workflow author as "An unexpected error occurred", and on guidance and load-related-record steps it did not reach them at all —withAiAssistre-tagged it and the step degraded to manual, permanently and silently.It now surfaces as
AiModelUnusableError, naming the model and the error class, classifiedconfiguration, and passing through the degrade path because a refused model is a permanent fault rather than the transient outage that path exists for. The provider's own sentence stays in the technical message: anAccessDeniedExceptionspells out the caller's IAM role ARN and AWS account id, which has no place in the Forest UI or the audit trail.Tests
apiKey; the region fallbacks and refusal; the per-provider CLI rules; the embeddedaioption; the Router boundary; the degrade-path interaction.bedrock-tool-choice.test.ts— thesupportsToolChoiceValuescontract against the real LangChain class, no mock and no AWS, with a negative control that goes green if the override is deleted.bedrock.integration.test.ts— the real Converse API. It fails in CI when the secrets are missing rather than skipping, because a silent skip reads as "Bedrock is verified". Locally it skips.It asserts each advertised line has a model the account can drive, then sweeps
ListFoundationModels∪ListInferenceProfiles. Entitlement and AWS's 30-day parking of a Legacy model are account states and are excluded; throttling is not — it is our own serial walk, so it fails the run rather than excusing it.The three CI secrets (
BEDROCK_AWS_REGION,BEDROCK_AWS_ACCESS_KEY_ID,BEDROCK_AWS_SECRET_ACCESS_KEY) are set.Notes for the reviewer
@langchain/awsis pinned to 1.4.3, the newest release still accepting@langchain/core ^1.0.0; 1.4.4+ require^1.2.8and the repo pins1.1.48.@aws-sdk/*packages into the Docker image, includingclient-kendraandclient-bedrock-agent-runtimewe never use — hard dependencies of the package, not prunable.docker/deps/yarn.lockis regenerated; the@forestadminclosure is unchanged.yarn start:with-executor:with-bedrockinpackages/_exampleruns the agent and executor against Bedrock locally.Definition of Done
General
Security
No new credential material is handled: Bedrock auth is delegated to the AWS SDK's own chain. Refusing
AI_API_KEYstops an operator believing a key is in use when it is not, and the provider's error message is kept out of the user-facing channel because it names the caller's IAM identity and AWS account.Note
Add Amazon Bedrock as a self-hosted AI provider in
workflow-executorbedrockto the AI provider union and constructs Bedrock chat models viaChatBedrockConversewith zero retries, no API key, and AWS credential-chain authentication. Region resolution prefers explicit config, thenAWS_REGION, thenAWS_DEFAULT_REGION.AIModelNotAllowlistedError.Router.validateConfigurationsandProviderDispatcherreject Bedrock configurations on the server-side AI proxy. The embedded executor and CLI acceptAI_PROVIDER=bedrockwithoutAI_API_KEYand require a region.ResourceNotFoundExceptionandAccessDeniedExceptionfrom Bedrock asAiModelUnusableError, preventing permanent config errors from degrading to manual steps. Sensitive provider messages are kept separate from user-facing text.bedrock.integration.createBaseChatModelBedrock branch omits explicit credentials entirely — any deployment relying on static key auth for Bedrock will fail.isModelSupportingTools(model, 'bedrock')now applies the allowlist; callers passing a Bedrock model without the provider argument will get OpenAI rules and incorrect results.claude-3-sonnet-20240229is newly rejected for Anthropic.Changes since #1897 opened
isAiConfigurationErrorutil inworkflow-executorto classify BedrockValidationExceptionerrors as configuration errors when the error message matches/model identifier/iregex pattern [25fe21f]resolveBedrockRegionutil inai-proxyto use logical-OR operator instead of nullish coalescing when selecting the AWS region [25fe21f]ai-proxyverifyingapiKeyparameter validation, model identifier validation errors, and region resolution with emptyAWS_REGIONenvironment variable [25fe21f]workflow-executorfor BedrockValidationExceptionclassification as configuration error when message references model identifier [25fe21f]ai-proxywithout changing test behavior [25fe21f]bedrock.integration.test.tswithin theai-proxypackage [037198e]OPENAI_UNSUPPORTED_PATTERNSconstant inai-proxypackage [0a5e52c]Macroscope summarized 032024e.