fix(export): make generated Strands runtimes deployable - #2154
fix(export): make generated Strands runtimes deployable#2154aidandaly24 wants to merge 5 commits into
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2154 +/- ##
============================================
- Coverage 97.22% 97.19% -0.03%
============================================
Files 507 507
Lines 33809 33953 +144
============================================
+ Hits 32872 33002 +130
- Misses 937 951 +14 ☔ 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, tightly-scoped fix. I walked through the template updates, the exporter changes, and the new EC2-backed VPC lookup and didn't find anything that needs to change before merge. A few observations, none of which are blockers:
HarnessClient.resolveVpcIdFromSubnetsis wired cleanly throughAwsClients/CoreClientand the ec2 client is cached like the others; the new tests cover the multi-VPC / no-VPC / happy paths without mocking anything below the SDK boundary. 👍- The switch to a strict ARN regex in
parseHarnessArnis the right call, and the behavior change of always trusting the ARN's region (rather than falling back to CLI region) is now consistent betweenharnessIdFromArnandregionFromHarnessArn. The test inharness.test.ts(arn:aws:lambda:...) and the partition test inserviceHarness.test.ts(arn:aws-cn:...) cover the important edges. - The
main.pytemplate'slimits = { … } or Noneidiom is intentional: whenhasExecutionLimitsis true solely becausetimeoutSecondsis set, both{{#if}}branches inside the dict literal are stripped, so it evaluates toNone. As long asagent.stream_async(..., limits=None, ...)is accepted by strands-agents 1.54 (per the verification notes in the PR description, it is), this is fine. serviceHarness.tsnow surfaces$unknownunion members as export notes instead of silently dropping them (skills, memory, environment, environment artifact, filesystem configs). Good coverage inserviceHarness.test.ts.- Header credential names / python function names now hash their inputs, so
X-Api-KeyvsX_Api_Keyno longer collide — nice; that's exercised by the new "keeps normalized header names distinct" test. readStrandsVersionregex was updated to tolerate the newstrands-agents[extras]form.- No new features here that would need telemetry instrumentation; existing plumbing is untouched.
LGTM to merge.
ed3b929 to
bee4e65
Compare
|
Claude Security Review: no high-confidence findings. (run) |
| // results to. CloudWatch is a distinct service from the AgentCore data plane, | ||
| // so it gets its own client/factory rather than reusing `data`. | ||
| logs(config: ClientConfig): CloudWatchLogsClient; | ||
| ec2(config: ClientConfig): EC2Client; |
There was a problem hiding this comment.
Why do we need an EC2 client?
| .send(new GetHarnessCommand({ harnessId: id })); | ||
| } | ||
|
|
||
| async resolveVpcIdFromSubnets(subnetIds: string[], options: CoreOptions): Promise<string> { |
There was a problem hiding this comment.
Why do we need to do this? The API doesn't require this: https://docs.aws.amazon.com/bedrock-agentcore-control/latest/APIReference/API_NetworkConfiguration.html
| spec.networkConfig && | ||
| !spec.networkConfig.vpcId | ||
| ) { | ||
| spec.networkConfig.vpcId = await config.core.harness.resolveVpcIdFromSubnets( |
There was a problem hiding this comment.
I'm not sure this need to be in scope. The main thing is generating the code that the user can then use. The user can also configure VPC stuff on their own.
Summary
strands-agents 1.54.xAPIs and provider extrasEXPORT_NOTES.mdinstead of silently dropping themVerification
Source gate
bun install --frozen-lockfilebun test src: 2,573 pass, 0 fail, 186 filesbun run typecheckbun run lint:checkbun run format:checkbun run buildbun run secrets:checkGenerated Python matrix
Generated real exports and ran
uv sync,compileall, andimport mainfor:All five resolved
strands-agents 1.54.0and imported successfully. OpenAI/Mantle resolvedopenai 2.54.0, Gemini resolvedgoogle-genai 2.20.0, and LiteLLM resolvedlitellm 1.96.0. A generated-module probe invoked the Bedrock entrypoint twice and verified fresh cancellation signals plus identical per-invocation limits (LIMITS_OK).Live AWS
Account
603141041947, regionus-east-1, profiledeploy:ExportHarnessFix0831_source-jBo3nhtmm2to READY and invoked itus-west-2, proving ARN-region precedence)ExportHarnessFix0831_exported_agent-hLU1ha80fdExportHarnessFix0831_exported_arn_agent-ItzBMG47U0end_turn, and the requested markerExportHarnessFix0831_mcp_runtime-6ZSyRO2yMEMCP_CONTEXT_OKThe current refactor deploy path consumes project credentials as references rather than provisioning them, so the exporter now states that the named provider must exist instead of claiming deploy creates it.
Cleanup
AgentCore-ExportHarnessFix0831-default: deleted