Conversation
A credential chain that resolves nothing used to surface on the first AI step of the first workflow, long after the instance answered its health check. The common cause is an AWS profile mounted where the image's user cannot read it: the container runs as `node`, so a profile at /root/.aws is never found. Runner.start() now resolves the chain once, right after the agent probe and before the run store opens — the same fail-fast reasoning already applied to AWS_REGION. A chain that does not answer within 10s only warns: on EC2 and ECS it ends in a metadata call that can lag, and a slow one must not cost a deploy. This proves credentials were found, not that they may call Bedrock; a policy missing bedrock:InvokeModel still surfaces on first use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (7)
🛟 Help
|
…l probe @smithy/credential-provider-imds caps itself at DEFAULT_TIMEOUT 1000ms with DEFAULT_MAX_RETRIES 0, so the metadata lag the race guarded against cannot happen. The only unbounded path left is a profile whose own credential_process hangs, which the comment now names instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
…by bearer token Findings from the review pass on this PR, verified against @langchain/aws 1.4.3: - With AWS_BEARER_TOKEN_BEDROCK set, LangChain builds the sigv4 credential provider beside the bearer one, and it rejects by design. The probe would have refused to boot a deployment that works today. Skipped when a token is present. - The catch claimed one diagnosis for every failure: an MFA prompt, an expired SSO session or a denied AssumeRole now keep their own message instead of being told to check Docker file permissions. The cause is attached rather than flattened into a string. - The comment justifying the unbounded await was wrong. IMDS and the ECS provider cap themselves; the STS legs (IRSA/web-identity, role_arn, SSO) set neither request nor connection timeout, so the cap is back with the real reason. - probeCredentials is required on the port: the doubles it was made optional for bypass structural checking anyway, so the ? only let a production adapter drop a boot gate silently. ServerAiAdapter and AlwaysErrorAiModelPort say no-op out loud, and Runner logs the probe like the agent one. - The probe raises a boundary error, not AiModelPortError, per the hierarchy in CLAUDE.md: a container misconfiguration is not a transient AI outage. - Typed as ChatBedrockConverse instead of a structural cast, so a release moving client fails the build rather than turning the probe into a silent no-op, with a contract test against the real dependency beside bedrock-tool-choice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
| ): Promise<{ tools: RemoteTool[]; failures: McpServerLoadFailure[] }>; | ||
| // Boot gate, resolved before the executor reports itself ready. An implementation with no | ||
| // credentials to check returns immediately — explicitly, so a missing one is a compile error. | ||
| probeCredentials(): Promise<void>; |
There was a problem hiding this comment.
🟠 High ports/ai-model-port.ts:23
Runner.start() crashes with probeCredentials is not a function when a JavaScript consumer supplies a pre-existing custom aiModelPort without this method. Make probeCredentials optional and guard the call so custom or no-credential ports continue to start normally.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/workflow-executor/src/ports/ai-model-port.ts around line 23:
`Runner.start()` crashes with `probeCredentials is not a function` when a JavaScript consumer supplies a pre-existing custom `aiModelPort` without this method. Make `probeCredentials` optional and guard the call so custom or no-credential ports continue to start normally.
There was a problem hiding this comment.
Fixed in 94be9e0 — the scenario is real, and it collides with a finding from another reviewer that argued the opposite. Both are right about their own case, so I kept the type and guarded the call.
Why the type stays required: aiModelPort is built in build-workflow-executor.ts:117-125 from exactly three classes, and two of them — ServerAiAdapter and AlwaysErrorAiModelPort — did not implement the method while it was optional. That compiled, and a boot gate any adapter can decline without saying so is worth less than no gate at all: nothing would have told us. Making it required turned that into two compile errors and two explicit one-line no-ops.
Why your scenario still holds: Runner and RunnerConfig are exported (index.ts:126-127), so an untyped caller can construct one with a port predating this method. TypeScript protects our adapters, not them.
So the call site is now probeCredentials?.() against a required type. A TS implementation that omits it fails the build; a JS consumer that omits it degrades rather than crashing on a check they never asked for. Pinned by a test that hands Runner a port with the method deleted and asserts it reaches running.
One correction to the report while I am here: the crash message would have been probeCredentials is not a function at boot, not a silent failure — but a loud crash in someone else's deployment is still their outage, so the point stands.
…odelPort Runner and RunnerConfig are exported, so a JavaScript caller can hand us a port built before probeCredentials existed. The port keeps it required — that is what stops one of our own adapters dropping the boot gate silently — but the call site guards, so a consumer degrades instead of crashing on a check they never asked for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The timeout test went out with the timeout and never came back when it did, so the cap that stands between a blackholed STS egress and a boot that never finishes had no test at all. Restored, plus the two adapter no-ops the required port method introduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new probeCredentials case declared its own, shadowing the outer one and failing lint. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 participant
Follow-up to #1897, from a gap found while documenting the Docker path.
The problem
AWS credentials that resolve to nothing were only discovered on the first AI step of the first workflow — long after the container started and passed its health check. The executor reports itself healthy, the deploy looks clean, and the failure lands on an end user running a workflow.
The common cause is our own image. It runs as
USER node, so the AWS SDK reads/home/node/.aws. Mounting a profile to/root/.aws— the reflex, and what most Docker examples show — resolves nothing. Measured against the built image:The fix
Runner.start()resolves the chain once, right after the agent probe and before the run store opens. Same reasoning already written intocli-core.tsforAWS_REGION:That argument applies identically to credentials, and this PR finishes the job.
A slow chain does not fail the boot. On EC2 and ECS the chain ends in a local metadata call that can lag, so a probe that does not answer within 10s logs a warning and starts anyway. Trading a rare misconfiguration for a flaky deploy would be a bad deal.
What it proves is narrow, and the message says so: credentials were found, not that they may call Bedrock. A policy missing
bedrock:InvokeModelstill surfaces on first use.Shape
AiClient.probeCredentials()— resolves the chain for every bedrock configuration, no-op for the others. It reaches into theChatBedrockConverseclient on purpose: re-deriving the chain would test a different object than the one production calls.AiModelPort.probeCredentials?()— optional, so the 40-odd test doubles of this port stay valid. Same pattern asloadToolsWithFailuresinai-client.ts.Runner.start()calls it; absence means nothing to verify.Covers both entry points: standalone and embedded both go through
Runner.start().Tests
Four in
ai-client.test.ts(no credentials → refuses; resolves → passes; times out → warns and starts; non-AWS provider → no-op) and two inrunner.test.ts(probe failure leaves the run store untouched and the runneridle; a port without the method starts normally). Verified failing without the change: 2 of the 4 and 1 of the 2 go red when the probe is neutered.Reviewer notes
ENOSPCon the build host, not a code failure. The path itself was measured on the previous image, as quoted above./home/node/.awsexplicitly. This PR makes that note a safety net rather than the only defence.🤖 Generated with Claude Code
Note
Verify AWS Bedrock credentials at startup in
Runner.startprobeCredentialsmethod toAiModelPortand calls it duringRunner.startbefore run-store initialization; a rejected probe aborts startup.AiClient.probeCredentialsconcurrently probes all Bedrock configurations, skips bearer-token providers, and races credential resolution against a 10-second timeout. Timeouts log a warning and allow startup to continue; resolution failures reject withAIBadRequestErrorpreserving provider-specific messages.AiCredentialProbeErrorto the workflow-executor public exports;AiClientAdapterconverts probe failures to this type with the original error as cause.AlwaysErrorAiModelPortandServerAiAdapterimplementprobeCredentialsas immediate no-ops.Runner.startuses optional chaining (port?.probeCredentials) so untyped runtime ports without the method continue startup, while the typed interface requires it. Bedrock credential failures that previously surfaced only at first model call now abort startup.Macroscope summarized 1e904f0.