Uh oh!
There was an error while loading. Please reload this page.
feat(project): implement project build - #1970
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| // single command compiles the app and then synthesizes it. Synthesis needs no | ||
| // AWS credentials: each stack's environment comes from aws-targets.json. | ||
| yield { message: "Synthesizing CloudFormation templates" }; | ||
| await this.run(["npm", "run", "cdk", "--", "synth", "--quiet"], cdkDir); |
There was a problem hiding this comment.
Are we not using @aws-cdk/toolkit-lib anymore?
There was a problem hiding this comment.
For now, I went with a subprocess to keep the PR small. using toolkit-lib means porting the wrapper and the schema pinning first, and deploy is what actually needs those. So I will introduce it when I introduce deploy. It should be easy to switch later, build() already yields events so it's a 2-way door decision
| return project; | ||
| } | ||
| public async *build(project: Project): AsyncGenerator<ProjectEvent, void> { |
There was a problem hiding this comment.
How does this build impl handle alternative project backends like terraform and SDK if we chose to implement those in the future?
There was a problem hiding this comment.
In the commit I just pushed, we use the managedBy field in agentcore.json
| handle: async (ctx) => { | ||
| // withProject has already resolved the enclosing project. | ||
| const project = ctx.require(ProjectKey); | ||
| // Progress goes to stderr, keeping stdout for machine output. Subprocess | ||
| // output goes to the debug log; on failure ProcessFailedError carries it. | ||
| for await (const event of config.projectManager.build(project)) { | ||
| config.io.stderr.write(`${event.message}\n`); | ||
| } | ||
| config.io.stderr.write(`Built project '${project.name}'\n`); |
There was a problem hiding this comment.
I like that this is kept clean. It doesn't need to be aware of the build backend or any of the other steps.
| await this.checkTool("npm", "Install Node.js: https://nodejs.org/"); | ||
| // The generated package.json defines `cdk` as "npm run build && cdk", so this | ||
| // single command compiles the app and then synthesizes it. Synthesis needs no | ||
| // AWS credentials: each stack's environment comes from aws-targets.json. | ||
| yield { message: "Synthesizing CloudFormation templates" }; | ||
| await this.run(["npm", "run", "cdk", "--", "synth", "--quiet"], cdkDir); |
There was a problem hiding this comment.
I think we had discussed that build would:
- Validate Schemas
- Generate ZIP artifacts for CodeZIP Agents
- Run CDK Synth
I'm not sure if Generate ZIP artifacts for CodeZIP Agents is happening here or if we decided to move that to a different PR.
There was a problem hiding this comment.
All three are happening. schema validation is up front, withProject resolves the project through ProjectSpecSchema, so build can't run on an invalid agentcore.json (#1972 makes that catch the CodeZip runtimeVersion rule and name the field).
the ZIP is generated by synth: the construct library's packager runs uv pip install and stages the asset, so cdk.out gets the zip and the template's CodeConfiguration.Code.S3 points at it.
I validated and checked a real synth to be sure. so there's no separate ZIP step to write, doing our own would duplicate the packager deploy relies on. LMK what you think
305bf56 to
639227fCompare78097a8 to
0b2da99Compare
aidandaly24
left a comment
There was a problem hiding this comment.
This looks good to me one really minor comment:
| // The generated package.json defines `cdk` as "npm run build && cdk", so this | ||
| // single command compiles the app and then synthesizes it. Synthesis needs no | ||
| // AWS credentials: each stack's environment comes from aws-targets.json. |
There was a problem hiding this comment.
Non-blocking and fine as a follow-up: synthesis succeeds without credentials, but AWS_PROFILE currently causes the pinned ConfigIO.readAWSDeploymentTargets() to call STS even when every target already has an account. I confirmed this by redirecting STS locally. The build still succeeded, but made six GetCallerIdentity attempts and inherited the retry latency. Could we avoid that fallback when account values are already present so build is fully offline?
There was a problem hiding this comment.
creating a follow up issue for this. thanks for catching this!
`agentcore project build` compiles the project's CDK app and synthesizes its CloudFormation templates, so the deployable artifacts exist before deploy. Synthesis runs offline: each stack's environment comes from aws-targets.json, so no credentials are needed. An empty targets file makes the CDK app fail with its own actionable message rather than the CLI guessing an account. The generated package.json defines `cdk` as "npm run build && cdk", so one `npm run cdk -- synth --quiet` covers both compile and synthesis. Also puts withProject to work for the first time: it resolves the enclosing project and hands it to the handler through ProjectKey, and it wraps only build so that `create` (which refuses to nest inside a project) stays unaffected. Its cwd is now resolved per invocation instead of at wiring time, so the directory the user actually ran in is the one searched.
The CDK construct library rejects a CodeZip runtime that declares no
runtimeVersion ("runtimeVersion is required for CodeZip builds"), and it is the
field that selects the packager. Without it, synthesizing a freshly created
python project fails on its own scaffolded config.
Container builds take their version from the image, so the container template
is unaffected.agentcore.json already records `managedBy` (CDK is the only value today), but nothing read it: build() hardcoded the CDK path, so adding a terraform or no-IaC backend later would have meant editing that path instead of adding alongside it. Carry managedBy on Project and switch on it in build(), delegating the CDK work to a private buildWithCdk(). The default arm assigns to `never`, so a new ManagedBySchema member fails to compile until it has an arm here.
0b2da99 to
05c244eCompareUh oh!
There was an error while loading. Please reload this page.
agentcore project buildcompiles the project's CDK app and synthesizes its CloudFormation templates, so the deployable artifacts exist before deploy.The generated
package.jsondefinescdkas"npm run build && cdk", so that one command covers both compile and synthesis — no need to construct anode_modules/.bin/cdkpath or branch onwin32.Synthesis is credential-free, by design
An earlier draft of this called STS
GetCallerIdentityand wroteagentcore/aws-targets.jsonas a side effect ofbuild. That isn't needed: each stack takes an explicitenv: { account, region }fromaws-targets.jsonand nothing in the app callsfromLookup, so synth never talks to AWS. The account is only load-bearing at deploy.So
buildstays offline and does not write config. Whenaws-targets.jsonis still[], the CDK app raises its own error, which already says what to do:wrapped by
ProcessFailedErrorwithFix the issue and run 'cd .../agentcore/cdk && npm run cdk -- synth --quiet' to retry.Deliberately not guessing the account also avoids a trap the earlier draft had: it wrote
regionunvalidated, butAgentCoreRegionSchemais an enum of 9 regions.--region eu-west-2would have written a region synth then rejects, and because the CLI's own re-read usedz.array(z.unknown())it would see a non-empty list and never repair the file.withProject goes into service
src/middleware/withProject.tsxalready existed but was never exported and never used. It now resolves the enclosing project and hands it to the handler viaProjectKey, so the handler body is just:Two changes to it:
cwdis optional and resolved per invocation (config.cwd ?? process.cwd()) rather than captured at wiring time, so the directory the user actually ran in is the one searched.ProjectStateErrornaming the path searched and the file looked for, and pointing atagentcore project create.It wraps only
build, not the whole router —createrefuses to nest inside an existing project, so requiring one would break it.Second commit:
runtimeVersionon the CodeZip templateIncluded here rather than as a follow-up, so
buildis never merged in a state where it fails on the config the CLI itself just scaffolded.The CDK construct library's schema refines
build !== 'Container' && !runtimeVersionintoruntimeVersion is required for CodeZip builds, and ourhello-world-pythontemplate didn't set it. Five lines:runtimeVersion: "PYTHON_3_14"on the template runtime plus the matching test assertion. Container builds take their version from the image, so the container template is untouched.A follow-up will mirror that refinement into our own
AgentEnvSpecSchema, which currently marksruntimeVersionplain.optional()— that way the CLI catches this class of error itself instead of surfacing CDK's zod path error.Verification
Beyond unit tests, I ran this against a real scaffolded project with a real
npm install:aws-targets.json→ the CDK app's "No deployment targets configured" error, wrapped with the retry hint.AWS_ACCESS_KEY_ID/SECRET/SESSION_TOKENblanked andAWS_PROFILEpointed at a nonexistent profile →Built project 'Demo', emittingcdk.out/AgentCore-Demo-dev.template.json. Confirms synth needs no credentials.runtimeVersionback out of that same project → reproduces the CodeZip failure, which is what motivated the second commit.Unit coverage: the exact synth command and cwd, the missing-
node_moduleserror, subprocess failure propagation, the progress event, resolution from a nested directory, and failing outside a project.bun test— 1061 pass, 0 failtsc --noEmitclean,oxlintclean