fix(import): translate Bedrock Agents into owned runtime code instead of proxying them - #2153
Conversation
|
Claude Security Review: no high-confidence findings. (run) |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## refactor #2153 +/- ##
==========================================
Coverage 97.14% 97.14%
==========================================
Files 519 526 +7
Lines 35492 36458 +966
==========================================
+ Hits 34478 35418 +940
- Misses 1014 1040 +26 ☔ 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
Reviewed the eight fixes against bedrockAgent.ts, importBedrockAgent.ts, templates/runtime.ts, and the Python proxy template. The changes are well-scoped, all failure modes are covered by tests at real I/O boundaries (control-plane client injection, in-memory describeBedrockAgent fakes), and the live-lifecycle validation in the description confirms the caller-owned-role and long-session-id paths end-to-end.
A few things I looked at and am comfortable with:
- Session-id normalization (
main.py): 64-char SHA-256 hex matches the^[0-9A-Za-z._:-]{2,100}$pattern, non-string/empty inputs correctly fall through to a fresh UUID, and emptycontext.session_idshort-circuits topayload.get("sessionId")as intended. - Response validation (
bedrockAgent.ts):agent.agentId/agent.agentNameare now checked beforegetAgentAlias, and the alias response is cross-checked against the requested IDs, which is what the description promised. additionalPoliciesomission withexecutionRoleArn(templates/runtime.ts): the generatedbedrock-agent-policy.jsonfile is preserved for the user to attach manually, and the README template branches onusesExistingExecutionRoleto match — I traced both branches throughimportBedrockAgentResolverand the rendered README.- Protocol pre-check in
add/runtime/index.tscorrectly runs beforeresolveImportBedrockAgentInput, so--protocol MCPno longer triggers a Bedrock describe call (verified by the newrejects a non-HTTP protocol before describing the agenttest).
One minor observation, not blocking: bedrockAgent.ts switched from await import("@aws-sdk/client-bedrock-agent") to a top-level static import. This is consistent with src/core/invokeRuntime.ts (which also statically imports an AWS SDK command class on a hot handler path), so it's aligned with existing convention — just noting the deviation from the previous lazy-load pattern in case CLI cold-start latency is being tracked.
Nice work.
| ) | ||
|
|
||
| response = client.invoke_agent( | ||
| response = await asyncio.to_thread( |
There was a problem hiding this comment.
Hah it looks like the AI didn't actually import this feature properly. It just proxies to Bedrock Agents. 😆
We'll have to implement this properly. I think we already have the code. We just need to port it over.
| input: DescribeBedrockAgentInput, | ||
| ) => Promise<BedrockAgentMetadata>; | ||
|
|
||
| export interface BedrockAgentControlClient { |
There was a problem hiding this comment.
What's the upshot of this interface?
3022a49 to
866509f
Compare
…ranslation `--type import` previously scaffolded a proxy runtime that forwarded every request to the source Bedrock Agent alias via bedrock-agent-runtime:InvokeAgent. That is not an import: the generated runtime could not be edited, stayed coupled to the source alias for its whole life, and broke if the alias moved or was deleted. Import now resolves the selected alias to its immutable agent version and translates that version into customer-owned Strands or LangGraph Python. The generated application invokes models and translated tools directly and keeps working after the source alias is removed. - Add src/core/project/bedrockAgentImport: an alias-pinned snapshot loader (GetAgentVersion, never GetAgent, so draft edits cannot leak in), paginated action groups / knowledge bases, recursively resolved collaborators with cycle protection, and Strands + LangGraph translators behind one interface. - Emit main.py, collaborator modules, pyproject.toml and IMPORT_NOTES.md through the project layer so writes, rollback and spec ownership stay in ProjectManager. - Record behaviour that cannot be translated cleanly (OpenAPI and Lambda action groups, user-input actions, guardrails, prompt overrides, required bedrock:Retrieve permissions) in IMPORT_NOTES.md instead of claiming it was preserved or generating IAM the deployment cannot attach. - Carry only customer-overridden ORCHESTRATION inference settings; Bedrock echoes its own internal defaults for a DEFAULT prompt, and copying those changes the generated agent's behaviour. - Reject an alias routing to the mutable DRAFT version before any further service call, which is what the built-in TSTALIASID test alias points at. - Retrieve each knowledge base in the region named by its own ARN. - Give only the root module the Runtime entrypoint; a collaborator module is imported by the root, so a second BedrockAgentCoreApp() there would register a second entrypoint at import time. - Keep generated state to the per-session agent cache the repository's own templates already use, with one asyncio.to_thread so synchronous framework work cannot block the Runtime event loop. - Remove the proxy template, the alias invocation policy, alias-readiness behaviour, Bedrock session-ID normalisation and the two-method BedrockAgentControlClient wrapper.
The README still told customers that `--type import` wraps an existing agent in a generated proxy. It also used TSTALIASID in its example, which routes to the mutable DRAFT version that import now rejects, so the documented command would have failed.
866509f to
db65b11
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
…s agent id Importing a supervisor agent silently produced a bare agent: no collaborator modules, no tools, and two misleading "creates a cycle" notes naming the supervisor's own id. On a ListAgentCollaborators summary, `agentId` and `agentVersion` describe the supervisor that owns the collaboration, not the collaborator. Only `agentDescriptor.aliasArn` identifies the collaborator. Reading `agentId` resolved every collaborator to its own parent, which the visited set had already seen, so each one was dropped as a false cycle. Pre-refactor `main` read only the alias ARN; the fallback was introduced during the port. Collaborators are now resolved from the alias ARN, and their alias is resolved to an immutable version the same way the requested agent's is, through one shared helper that also rejects DRAFT. The two existing collaborator tests passed because their fixtures put the collaborator's id in `agentId`, which the service never does. Both now use the real shape, and a new test asserts the verbatim two-collaborator response observed live. Verified live: importing a supervisor with billing and weather collaborators generated both modules, wired them as tools, and after deployment the supervisor returned "SUPERVISOR: ... BILLING: ..." in one reply, so the generated collaborator module executed.
|
Claude Security Review: no high-confidence findings. (run) |
…models
Two parity gaps found by comparing against pre-refactor `main`.
`strands-agents-tools ~= 0.1.0` cannot be resolved at all. 0.1.x caps
strands-agents below 1.0, so it conflicts with `strands-agents ~= 1.15`, and the
0.1.9 wheel ships no `strands_tools.code_interpreter` module either. Every
Strands import of an agent with an AMAZON.CodeInterpreter action group therefore
produced a pyproject.toml that `uv sync` refused. Pinned to `~= 0.2.16`, which is
the floor mainline used. Verified by resolving and importing a generated
code-interpreter project.
langchain_aws derives the model provider from `model_id` and raises
`ValueError("Model provider should be supplied when passing a model ARN as
model_id")` when it starts with `arn`, which a Bedrock Agent may legally use for
provisioned throughput, custom models, and inference profiles. Mainline resolved
the provider with GetFoundationModel and always emitted `provider=`, defaulting
to "anthropic" when the lookup failed. Rather than add that call and guess a
provider, the import now records the ARN and the required `provider=` argument in
IMPORT_NOTES.md, consistent with how every other untranslatable input is handled.
|
Claude Security Review: no high-confidence findings. (run) |
…aborator history
Two capabilities pre-refactor `main` had that the port dropped.
langchain_aws refuses to infer a provider from any `arn:` model id and raises
`ValueError("Model provider should be supplied when passing a model ARN as
model_id")`, so a LangGraph import of an agent using a foundation-model,
provisioned-model, or inference-profile ARN generated code that failed at module
import. A foundation-model ARN names its provider in the resource
(`.../foundation-model/anthropic.claude-...`), so that is now parsed and emitted
as `provider=`, matching what mainline resolved through GetFoundationModel but
without the extra service call or its "anthropic" fallback guess. Provisioned and
inference-profile ARNs cannot be resolved offline, so those emit a commented
`provider=` at the call site and an IMPORT_NOTES entry. Verified by executing each
generated ChatBedrock call against langchain_aws 1.7.5.
`relayConversationHistory: TO_COLLABORATOR` means the source agent forwarded its
conversation to that collaborator. Mainline generated code for it; the port
reduced it to a note, which was wrong: it was a supported behaviour, not an
unsupported one. Strands now uses `@tool(context=True)` with a `ToolContext`
parameter, which the tool schema hides from the model, and LangGraph uses
`Annotated[dict, InjectedState]`. Collaborator modules take an optional
`relayed_messages`; only a relaying parent passes it. A `DISABLED` collaborator is
generated exactly as before.
Verified live: a supervisor whose collaborator relays was imported and deployed.
The codename BLUEHERON was told only to the supervisor in turn one; in turn two
the collaborator answered "RECALL: BLUEHERON". A fresh session with nothing
planted returned "RECALL: NONE_FOUND", so the answer came from the relayed
conversation and not from collaborator state.
|
Claude Security Review: no high-confidence findings. (run) |
Context
PR #2146 added
project create --type importandproject add runtime --type import. As merged,--type importscaffolded a proxy: a generatedmain.pythat calledbedrock-agent-runtime:InvokeAgenton the source alias for every request. This PR originally hardened that proxy against eight post-merge defects.Review feedback pointed out that the proxy is the wrong feature. The pre-refactor CLI on
mainimplements import as a one-time translation: it reads the agent definition, action groups, knowledge bases, guardrails, model settings, and collaborators, then generates runnable Strands or LangGraph Python that the customer owns and edits (src/cli/aws/bedrock-import.ts,src/cli/operations/agent/import/*, documented indocs/frameworks.md). The proxy kept the source alias on the request path permanently, generated no prompts or tools, and broke if the alias moved or was deleted.This PR was reworked to restore that contract. It does not change
--type create.What changed
--type importnow resolves the selected alias to its immutable agent version and translates that version into customer-owned Python. The generated application invokes models and translated tools directly and keeps working after the source alias is deleted.GetAgentVersion, neverGetAgent, so mutable draft edits cannot leak into the snapshot. Action groups, knowledge bases, and collaborators are read at that same version, paginated, with cycle protection.--framework strands|langgraph(defaultstrands) and the existing--memoryoptions both apply to import, shared identically byproject createandproject add runtime.FsTreeNode/ProjectManager, so writes, rollback, dependency installation, and spec ownership stay in the project layer.IMPORT_NOTES.mdrather than silently claimed: OpenAPI and Lambda action groups, user-input actions, Strands guardrails, custom prompt templates, collaborator session scope, and the exactbedrock:Retrievepermissions the execution role needs. OpenAPI actions, Lambda executors, user-input actions, relayed collaborator history, and generated IAM policy documents are deliberately not implemented.BedrockAgentControlClientwrapper, replaced by a singleCoreBedrockAgentImporter.import(request) -> planseam.Five of the eight defects the original PR fixed were properties of the proxy and are eliminated by removing it: alias-readiness checks, the caller-owned-role policy, and Bedrock session-ID normalization no longer have anything to apply to. Three are retained: service-response validation, protocol validation before any AWS call, and payload-shape validation. The region list is retained and completed, since the earlier fix still omitted
ap-northeast-2andus-east-2.Bugs fixed while porting
1. Bedrock's own defaults were copied into generated code
GetAgentVersionechoes the service's internal orchestration template and inference settings, for exampletemperature: 1and a</answer>stop sequence, even when the customer never overrode them. It also populatesadditionalModelRequestFieldson the ORCHESTRATION prompt of a completely default agent. Copying any of that makes service internals the generated agent's behavior, and the last case produced anIMPORT_NOTES.mdentry telling the customer to migrate prompt customization that did not exist. The loader now reads prompt and inference settings only whenpromptCreationModeorparserModeisOVERRIDDEN; otherwise the agent instruction is the system prompt.2. Collaborator modules registered a second Runtime entrypoint
Each generated collaborator module created its own
BedrockAgentCoreApp()and@app.entrypoint, and the root module imports it, so a second entrypoint registered at import time. Only the root module owns the entrypoint now.3. Collaborator follow-up was never reported
Notes were collected for the root agent only, so a collaborator's unimplemented action groups and knowledge-base permissions never reached
IMPORT_NOTES.md. Notes now cover the whole agent tree and name the collaborator. A collaborator-only code interpreter also produced an import with no matching dependency; thepyproject.tomldependency is now computed across the tree.4. The default test alias failed with an unmapped service error
TSTALIASIDroutes to the mutableDRAFTversion, which has noGetAgentVersionrepresentation. Import now rejects it before any further call and explains how to create a version and an alias. The README example usedTSTALIASIDand would itself have failed; both it and the--agent-alias-idhelp text now state the constraint.5. Cross-region knowledge bases were retrieved from the wrong region
Retrieval used the agent's region. It now uses the region in the knowledge base's own ARN.
6. Importing a supervisor agent dropped every collaborator
A supervisor agent imported as a bare agent: no collaborator modules, no tools, and one misleading
collaborator-cyclenote per collaborator naming the supervisor's own agent id.On a
ListAgentCollaboratorssummary,agentIdandagentVersiondescribe the supervisor that owns the collaboration, not the collaborator; onlyagentDescriptor.aliasArnidentifies the collaborator. ReadingagentIdresolved every collaborator to its own parent, which the visited set had already seen, so each was discarded as a false cycle. Pre-refactormainread only the alias ARN — the fallback was introduced during the port.Collaborators are now resolved from the alias ARN, and their alias is resolved to an immutable version the same way the requested agent's is, through one shared helper that also rejects
DRAFT.Both existing collaborator tests passed against the broken code because their fixtures placed the collaborator's id in
agentId, which the service never does. Both now use the real shape, and a new test asserts the verbatim two-collaborator response observed live.7. Generated dependencies were unbounded
Dependencies used
>=with no upper bound, and one was unpinned entirely, so a major release could break an imported project. They now use~=like the scaffolded templates, bounded at the minor rather than the patch: copying the templates' patch-level pin exactly would ship a broken project, becauselanggraph ~= 1.0.2resolves to 1.0.10, whoselanggraph-prebuiltimports a symbol its ownlanggraph.runtimedoes not export.Validation
Automated
bun test srcbun run typecheckbun run lint:checkbun run format:checkbun run secrets:checkbun run buildGenerated Python
none,longAndShortTerm), and shape (with collaborators, tools, and knowledge bases; and bare).uv syncand imported them, including the memory module.InvokeAgentcall path and exactly oneBedrockAgentCoreApp/@app.entrypointper project.Live AWS lifecycle
Two full deployments in account
603141041947,us-east-1, against this branch's bundled CLI: one Strands import with--memory none, and one LangGraph import with--memory longAndShortTerm. Each used a source Bedrock Agent whose instruction demanded a fixed token prefix, so a correct reply proves the instruction survived translation.--agent-alias-id TSTALIASIDwas rejected with the new guidance before any further service call, and--region eu-north-1was rejected locally.InvokeAgentcall path, and the runtime spec carried noadditionalPolicies.GetAgentAliasreturnedResourceNotFoundException, and invoked again successfully. Every response began with the token the source agent's instruction required, so the instruction survived translation.[]and{}payloads returned a controlled validation error without reaching the model.The LangGraph run additionally exercised generated memory, which no earlier validation reached:
MEMORY_*_IDenvironment variable the generated code reads.userIdwas supplied in the payload.thread_id; a second session did not see the first session's facts.A third deployment covered multi-agent collaboration, importing a supervisor with a
billingcollaborator (relayTO_COLLABORATOR) and aweathercollaborator:BedrockAgentCoreApp/@app.entrypointexisted across the project, inmain.py.billing, matching the source configuration.SUPERVISOR: ... BILLING: ...), so the generated collaborator module executed rather than being inlined or ignored.After all three runs the stack, runtime, memory, source agents, aliases, and IAM roles were confirmed deleted.
Not exercised against AWS, and unit-tested only: knowledge-base retrieval, action-group stubs, code interpreter, and guardrails.
Out of scope
project create --type createbehaviour is unchanged.IMPORT_NOTES.md.bedrock-agentcore ~= 1.9.1, which resolves to exactly 1.9.1. That affects five templates and is left alone here.