feat(templates): wire up non-bedrock model providers to templates - #2175
Conversation
…templates at resolution
…op redundant hasIdentity
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## refactor #2175 +/- ##
==========================================
Coverage 97.12% 97.12%
==========================================
Files 535 536 +1
Lines 36844 36951 +107
==========================================
+ Hits 35783 35890 +107
Misses 1061 1061 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice cleanup — the schema/flag translation, resolveModelProviderScaffold, and pyproject conditionals hang together well, and the tests exercise the create/add-runtime flows against real temp directories with meaningful assertions on agentcore.json + .env.local. A few observations, none blocking:
create/index.tsstill usesz.enum(["bedrock", "open_ai", "gemini", "lite_llm", "anthropic"])(strict snake_case) whileadd runtimeaccepts the case-insensitiveModelProviderSchema. Given the flag description already documents both variants explicitly, this is acceptable, but worth noting as a minor UX seam if you unify later.- The A2A python-strands
pyproject.tomlwas updated for provider-specificstrands-agentsextras (which is fine, since the A2A resolver already routes throughresolveModelProviderScaffoldandmodel/load.pyhas the provider branches), but the PR description says A2A is out of scope. Not an issue — just a heads-up that you're already essentially wired up for it there, minus a README refresh. .env.localwritten duringcreate(manager.tsx:181) has no rollback if a later step (harness scaffolding, install, git init) fails. This matches the existing "failed step leaves scaffolded files in place" comment, so it's intentional; just calling it out sinceaddResourcedoes have rollback wiring for the same file.
Good to merge.
0799a97 to
5247c78
Compare
|
Claude Security Review: no high-confidence findings. (run) |
notgitika
left a comment
There was a problem hiding this comment.
Good work! LGTM with the rough edges defined in the description let's have that as a follow up
Problem
non-bedrock model providers are not wired up with the existing templates.
Solution
project createbetween runtime and harness flows..env.localfor those values so that local dev works.Verification
Starting with a gemini key in my local .env file:
Notes
Non container python runtimes, and all TS runtimes require a user id (
X-Amzn-Bedrock-AgentCore-Runtime-User-Id) header since otherwise the workload access token used to fetch the api key is not injected.The python non-container handles this with a special case in the SDK.
The typescript does not. here
This is never experienced through the old CLI since we default this value
agentcore-cli/src/cli/aws/agentcore.ts
Line 57 in 543a6da
However, I'm not confident this is the right behavior, so I left it out here.
Right now it requires doing this:
i.e. providing the user id, and an explicit accept flag.
The accept flag is only required in TS SDK due to a check here: https://github.com/aws/bedrock-agentcore-sdk-typescript/blob/efef021a7a75aebefccb5111bc0427b5f4c1cbfb/src/runtime/app.ts#L371-L382, whereas Python ignores it completely and forces
text/event-stream.Python code that ignores it https://github.com/aws/bedrock-agentcore-sdk-python/blob/82eddf32c7134e549c630f1a29bb59697b7dd596/src/bedrock_agentcore/runtime/app.py#L573-L579.
Future Work
acceptparam for ts only.