Repository navigation
feat(compat): load v0 projects and delegate deployment - #2575
Conversation
There was a problem hiding this comment.
AgentCore Harness Review
Verdict: Looks good
Nice slice. The read-only compatibility path is well-scoped: v0 detection is a tiny pure function, translateV0Spec is pure (verified by a test) and uses structuredClone, write operations are uniformly gated via assertV1Project, and aws-targets.json is dispatched to the right schema based on project.legacy. Tests use real temp directories and the vendored schemas are properly isolated from lint/prettier and documented as byte-identical to upstream.
A few small things I noticed but don't think block merging:
FsProjectManager.resolvenow readsagentcore.jsonup to three times for a v0 project (v1 parse → header probe → v0 parse inloadLegacyProject). Harmless given file size, but if you want to tighten this,loadLegacyProjectcould accept the already-read raw object or you could parse the raw JSON once and dispatch.- In
translate.ts,mcpRuntimeTools && { toolRuntimes: ... }treats[]as truthy, so an emptymcpRuntimeToolsarray becomestoolRuntimes: []rather than being omitted. The dedicated test asserts omission only for theundefinedcase; if you want "no mcpRuntimeTools ⇒ no toolRuntimes" to also cover the empty-array case, switch tomcpRuntimeTools !== undefined(ormcpRuntimeTools?.length). Minor since downstream v1 code should treat the two identically. - The
assertV1Projectmessage hard-codes command strings like"remove all"and"export harness". Worth a quick confirmation thatnpx @aws/agentcore@0 remove all …andnpx @aws/agentcore@0 export harness …are the exact invocations v0 users should type; otherwise the guidance will send people down the wrong path.
Nothing here needs changes before merging.
|
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 #2575 +/- ##
============================================
- Coverage 97.50% 96.32% -1.19%
============================================
Files 641 670 +29
Lines 47480 51173 +3693
============================================
+ Hits 46297 49291 +2994
- Misses 1183 1882 +699 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
fb80c6a to
e716259
Compare
|
Claude Security Review: no high-confidence findings. (run) |
|
Claude Security Review: no high-confidence findings. (run) |
A frozen copy of aws/agentcore-cli@v0.31.1 src/schema reads unmigrated v0 projects, so anything v0 accepted is still accepted. Lint and Prettier skip it to keep it diffable against upstream. The schema-audit fixtures cover the edge cases v0 accepts and v1 doesn't.
projectFormat tells v0 specs (version 1) from v1 ones, and translateV0Spec maps a parsed v0 spec onto v1's shape: version 2, v0-only collections moved aside, mcpRuntimeTools renamed to toolRuntimes, artifact renamed to build, and capacityProviderVolume mounts dropped.
5ea4962 to
96e9ee3
Compare
|
right now, if a user inputs a deprecated command, it shows the generic error: we could add guidance to the user to use other commands or guide them to AWS docs as a follow up |
jariy17
left a comment
There was a problem hiding this comment.
We need to refactor this code a little by shifting legacy loose functions into the Project MAnager.
|
|
||
| return { | ||
| spec, | ||
| extras: { abTests, datasets, capacityProviders, unassignedTargets } as LegacyExtras, |
There was a problem hiding this comment.
First, abTests and unassignedTargets have been deprecated for a while. Let's just drop them. Also, what the is the goal of extras?
| export type ProjectFormat = "v0" | "v1"; | ||
|
|
||
| /** v0 CLIs write `version: 1`; anything else is validated as a v1 spec. */ | ||
| export function projectFormat(raw: unknown): ProjectFormat { |
There was a problem hiding this comment.
lets shift this into projectManager.
| import type { Project } from "../../../handlers/project/types"; | ||
|
|
||
| /** Guard operations whose implementation rewrites files using the v1 project shape. */ | ||
| export function assertV1Project(project: Project, command: string): void { |
There was a problem hiding this comment.
Follow Up: We should probably combine these loose functions into the ProjectManager if we can.
| env: { | ||
| ...process.env, | ||
| AGENTCORE_SKIP_CONFLICT_CHECK: "1", |
There was a problem hiding this comment.
The v0 deploy handoff drops --region. For example, agentcore deploy --region eu-west-1 --yes with AWS_REGION=us-east-1 still passes us-east-1 to the child. When no deployment targets exist, v0 uses that region to create the default target, so the first deployment can go to the wrong region. Existing targets use their saved region. Can we forward the resolved region to the child process and add a regression test for first deployment?
| @@ -0,0 +1,40 @@ | |||
| import { runInheritedProcess, type InheritedProcessRunner } from "../../../io"; | |||
|
|
|||
| export const V0_CLI_VERSION = "0.31.1"; | |||
There was a problem hiding this comment.
Lets pin this to @0 so we're not stuck upgrading this pin.
| @@ -0,0 +1,29 @@ | |||
| import { readdirSync, readFileSync } from "node:fs"; | |||
There was a problem hiding this comment.
This looks like a test related file. Can we call it xxx.test.ts
| @@ -0,0 +1,23 @@ | |||
| import { ProjectStateError } from "../../../errors"; | |||
There was a problem hiding this comment.
Can we inject this as middleware somehow?
There was a problem hiding this comment.
can be done as a follow up?
|
all comments will be addressed in a follow up |
|
Claude Security Review: no high-confidence findings. (run) |
aws#2575 and aws#2577 agentcore migrate replaces the preview and apply subcommands, following the Terraform apply model: it checks the project and prints the plan, then applies it. Applying lands next, so for now it prints the plan, says applying isn't available yet, and exits 0 when the plan is ready and 1 when it isn't. With aws#2575 merged, agentcore deploy deploys a v0 project with the v0 CLI, so the steps the plan suggests use it instead of npx @aws/agentcore@0. With aws#2577 merged, hyphenated target names are valid in v1, so they no longer block a migration.
aws#2575 and aws#2577 agentcore migrate replaces the preview and apply subcommands, following the Terraform apply model: it checks the project and prints the plan, then applies it. Applying lands next, so for now it prints the plan, says applying isn't available yet, and exits 0 when the plan is ready and 1 when it isn't. With aws#2575 merged, agentcore deploy deploys a v0 project with the v0 CLI, so the steps the plan suggests use it instead of npx @aws/agentcore@0. With aws#2577 merged, hyphenated target names are valid in v1, so they no longer block a migration.
Description
Adds compatibility mode for projects created by AgentCore CLI v0.31.1.
When v1 finds
agentcore/agentcore.jsonwith"version": 1, it validates the project with a frozen copy of the v0.31.1 schemas and translates it into the current in-memory model without modifying project files.The translated model keeps the v1 CLI and TUI available for
dev,invoke,status,logs, andtraces. Deployment is the exception: both directagentcore deployand the deploy step nested inside harnessagentcore devrun the pinned v0.31.1 deploy command from the project root. That preserves the complete v0 CDK application, prompts, validation, output, and exit code instead of wrapping the old project in v1 deployment logic.add,remove,build, andexportare blocked for v0 projects. Users can inspect and run the old project safely now, then use the separate migration workflow when it is available.Most of the diff is frozen v0 schemas and validation fixtures; the detection, translation, gating, and deploy handoff are comparatively small.
Testing done on a v0 project with v1 CLI:
devcommand (runtime) ranagentcore devin my project and this in the other terminal:devinspector modedeployingstatusagentcore logs runtimeFollow-ups:
Related Issue
Implements the compatibility execution model from the reviewed CLI v0-to-v1 proposal.
Documentation PR
Not applicable yet. Migration documentation will land with the migration workflow.
Type of Change
Testing
bun test— 4,003 passedbun run typecheckbun run lint:checkbun run format:checkbun run buildbun run secrets:checksrc/assets/was not modifiedbun auditcurrently reports the repository's existing transitivebraces@3.0.3advisory. This PR does not change dependencies orbun.lock.Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.