-
Notifications
You must be signed in to change notification settings - Fork 73
feat(project): implement project build #1970
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,7 +54,12 @@ export class FsProjectManager implements ProjectManager { | |
| const configPath = join(rootPath, "agentcore", "agentcore.json"); | ||
| try { | ||
| const spec = await this.json.read(configPath, ProjectSpecSchema); | ||
| return { name: spec.name, rootPath, runtimes: spec.runtimes }; | ||
| return { | ||
| name: spec.name, | ||
| rootPath, | ||
| managedBy: spec.managedBy, | ||
| runtimes: spec.runtimes, | ||
| }; | ||
| } catch (error) { | ||
| // A malformed agentcore.json is a user-correctable problem, not a crash. | ||
| if (error instanceof DeserializationError) { | ||
|
|
@@ -118,6 +123,45 @@ export class FsProjectManager implements ProjectManager { | |
| return project; | ||
| } | ||
|
|
||
| public async *build(project: Project): AsyncGenerator<ProjectEvent, void> { | ||
| // agentcore.json records which backend owns the project's artifacts. CDK is the | ||
| // only one today; a terraform or no-IaC backend adds an arm here rather than | ||
| // editing the CDK path. | ||
| switch (project.managedBy) { | ||
| case "CDK": | ||
| yield* this.buildWithCdk(project); | ||
| break; | ||
| default: { | ||
| // Exhaustiveness: a new ManagedBy member fails to compile until it is handled. | ||
| const unsupported: never = project.managedBy; | ||
| throw new ProjectStateError( | ||
| `project '${project.name}' declares an unsupported backend: ${String(unsupported)}`, | ||
| ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Compiles the generated CDK app and synthesizes its CloudFormation templates. | ||
| private async *buildWithCdk(project: Project): AsyncGenerator<ProjectEvent, void> { | ||
| const cdkDir = join(project.rootPath, "agentcore", "cdk"); | ||
|
|
||
| // The generated CDK app is built from its own node_modules; without them the | ||
| // failure would otherwise surface as an opaque "cdk: not found". | ||
| if (!existsSync(join(cdkDir, "node_modules"))) { | ||
| throw new ProjectStateError( | ||
| `CDK dependencies are missing for project '${project.name}'. ` + | ||
| `Run 'cd ${cdkDir} && npm install'.`, | ||
| ); | ||
| } | ||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Non-blocking and fine as a follow-up: synthesis succeeds without credentials, but
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. creating a follow up issue for this. thanks for catching this! |
||
| yield { message: "Synthesizing CloudFormation templates" }; | ||
| await this.run(["npm", "run", "cdk", "--", "synth", "--quiet"], cdkDir); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are we not using
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
Comment on lines
+156
to
+162
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we had discussed that build would:
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.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. All three are happening. schema validation is up front, the ZIP is generated by synth: the construct library's packager runs 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 |
||
| } | ||
|
|
||
| // Runs a command with its output streamed to the file logger. | ||
| private run(command: string[], cwd: string): Promise<void> { | ||
| return this.runner(command, { cwd, onOutput: (chunk) => this.logger.debug(chunk) }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,26 @@ | ||
| import { createHandler } from "../../../router"; | ||
| import { NotImplementedError } from "../../../errors"; | ||
| import { createHandler, ProjectKey } from "../../../router"; | ||
| import type { AppIO } from "../../../io"; | ||
| import type { ProjectManager } from "../types"; | ||
|
|
||
| export const createBuildProjectHandler = () => | ||
| type BuildProjectHandlerConfig = { | ||
| projectManager: ProjectManager; | ||
| io: AppIO; | ||
| }; | ||
|
|
||
| export const createBuildProjectHandler = (config: BuildProjectHandlerConfig) => | ||
| createHandler({ | ||
| name: "build", | ||
| description: "build the project's deployable artifacts", | ||
| handle: async () => { | ||
| throw new NotImplementedError("agentcore project build is not implemented yet"); | ||
| 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`); | ||
|
Comment on lines
+14
to
+24
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like that this is kept clean. It doesn't need to be aware of the build backend or any of the other steps. |
||
| }, | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the commit I just pushed, we use the
managedByfield in agentcore.json