feat: implement project deploy command - #2001
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor #2001 +/- ##
==========================================
Coverage 97.06% 97.06%
==========================================
Files 374 379 +5
Lines 22542 22874 +332
==========================================
+ Hits 21880 22203 +323
- Misses 662 671 +9 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| import { | ||
| BaseCredentials, | ||
| BootstrapEnvironments, | ||
| BootstrapStackParameters, | ||
| StackSelectionStrategy, | ||
| Toolkit, | ||
| type IIoHost, | ||
| type IoMessageLevel, | ||
| } from "@aws-cdk/toolkit-lib"; | ||
|
|
There was a problem hiding this comment.
this static import is increasing the binary size and startup latency by a lot. looking into solutions
|
increasing test coverage |
Deploys a project by synthesizing it, bootstrapping each target environment, then deploying every stack. Synthesis reuses build's code path so what deploys is what was just synthesized, and build's dependency check runs before anything touches AWS. Stack environments come from agentcore/aws-targets.json, which create() scaffolds empty. An empty list is an error naming the file to fill in rather than an account resolved from the active credentials, which would let deploy guess where the user's infrastructure belongs. Bootstrap is idempotent and no-ops quickly on a current environment, so it runs every deploy instead of probing CloudFormation first; --skip-bootstrap opts out. Targets sharing an environment are bootstrapped once. Bootstrap and deploy drive @aws-cdk/toolkit-lib in-process rather than shelling out to npx cdk, so progress arrives as structured messages and failures as typed errors instead of scraped stdout. src/io/cdk.ts adapts the toolkit's push-based IIoHost to the generator the manager pulls from, and is injectable so tests exercise deploy without reaching AWS. Deploy reads the cdk.out assembly synthesis just wrote instead of re-synthesizing, which both avoids a second synth and makes "deploy exactly what was synthesized" structural. A deploy runs for minutes, so those messages stream to stderr as they arrive: ProjectEvent gains an output variant carrying the toolkit's own wording, with debug and trace levels left in the debug log rather than on screen.
deploy shipped every stack in the assembly, so a project with a staging and a prod target reached both at once. It now takes --target, defaulting to "default", and bootstraps and deploys only that target. The target is resolved from aws-targets.json before synthesizing, so a misspelled --target costs no build and the error lists the configured names. Which stack belongs to the target comes from the synthesized manifest, matched on the agentcore:target-name tag the generated CDK app writes, rather than from the CLI reproducing that app's naming convention. The lookup runs before bootstrap so a mismatch fails in seconds, and the toolkit selects with PATTERN_MUST_MATCH so a name the assembly does not contain fails loudly instead of deploying nothing.
The toolkit is the heaviest dependency in the CLI and src/io/index.ts
re-exports runCdk, so a static import made every command load it:
agentcore --help ran in 2.7s from source, 0.7s once the import moved
inside the function that builds the toolkit. The compiled binary is
unchanged either way, since --compile embeds the module regardless.
src/io/cdk.ts splits into the three things a run does -- load the
toolkit, perform one operation with it, bridge its reporting to a
generator -- so each is reachable from a test. src/io/cdk.test.ts covers
them against the real toolkit package: constructing a Toolkit and its
BootstrapEnvironments, BootstrapStackParameters, and
StackSelectionStrategy helpers resolves no credentials and calls no API,
so the arguments a deploy passes are asserted against the values the
toolkit itself defines rather than stand-ins. What the tests assert
includes the ones a caller cannot see and a fake cannot check: that
messages are yielded while the operation is still running, that a
failure surfaces only after the output explaining it, that a request is
answered with its suggested default rather than prompting, and that
createCustomerMasterKey and PATTERN_MUST_MATCH reach the toolkit.
The fake in TestCoreClient still buffers rather than streams; it now
says so, and names the test that covers the real behaviour.
ProjectEvent becomes a discriminated union. It documented that exactly
one of step and output is set while typing both optional, which allowed
{} and both-at-once and spread `if (event.output)` checks through three
handlers. Both variants carry `message`, so a consumer that only writes
text needs no switch, and those checks are gone.
Bootstrap uploads a CloudFormation template that @aws-cdk/toolkit-lib ships as a file in its own package directory and finds at runtime relative to where that directory sits on disk. Bundling the package rewrote that lookup to the build machine's absolute path, so a released build reached a node_modules that only exists on the machine that built it: the npm bundle failed with ENOENT on the template, and the compiled executables, which have no node_modules at all, failed the same way. Nothing read that template until someone bootstrapped an account, so every build check passed. The bundle keeps the package external, as the pre-refactor CLI's esbuild config did, so node resolves a real @aws-cdk/toolkit-lib -- it is a declared runtime dependency, so installing the CLI installs it. That also returns the bundle to its previous size (32.8 MB back to 6.4 MB) and warm `agentcore --help` to ~0.5s, since the lazy import no longer has a second CDK toolchain inlined behind it to parse. A compiled executable cannot have externals, so the build embeds the template as an asset and bootstrap is pointed at a copy written out for it, in the one mode where the toolkit cannot find its own. The template is read from the installed package rather than vendored here, so it always matches the toolkit being compiled in, and compile fails if the package stops shipping it or if the executable does not carry its bytes -- the check the missing template needed and did not have. Verified on the artifacts: with the toolkit's own template moved aside, the executable bootstraps to the first AWS call, while the bundle under node fails on exactly the ENOENT above.
Hweinstock
left a comment
There was a problem hiding this comment.
I think this is one of the hardest pieces of functionality to implement in the project, but I think this is an awesome start. Had a few questions about ways we might be able to simplify, and maybe make it easier to support terraform in the future.
Also, maybe just nit on my end, but I find some of the comments overly verbose and distracting. However, if we see value in them expressing ideas not already expressed in the code, then I'm open to keeping them.
| // withProject has already resolved the enclosing project. | ||
| const project = ctx.require(ProjectKey); | ||
|
|
||
| // Progress and the CDK toolkit's own output both go to stderr, keeping stdout |
There was a problem hiding this comment.
does it make sense to avoid cdk specific language since we expect a terraform extension soon?
There was a problem hiding this comment.
true, editing the comment
| z.string().default("default"), | ||
| ), | ||
| flag( | ||
| "skip-bootstrap", |
There was a problem hiding this comment.
out of curiosity, when would a customer want to skip bootstrap? My understanding is that if an account/region is already bootstrapped it no-ops fairly quickly.
There was a problem hiding this comment.
when the region's already bootstrapped. because for us that isn't quite a no-op. we bootstrap with withExisting({ createCustomerMasterKey: true }), so re-running it updates the shared CDKToolkit stack and rewrites a staging bucket that had no CMK to use a customer-managed key. It also needs CloudFormation/IAM write access on every deploy, which a deploy-only role doesn't have. (I could recreate this that is why i'd added the flag).
but that's better handled by not bootstrapping than by a flag, so the flag is gone and we probe like main branch does: one DescribeStacks on CDKToolkit, read BootstrapVersion, require >= 30, bootstrap only if it's missing or older
| /** | ||
| * Something worth showing the user while a long-running project operation runs. | ||
| * | ||
| * A union rather than two optional fields, so an event is always exactly one of the |
There was a problem hiding this comment.
does this comment explain anything that isn't clear from the code?
There was a problem hiding this comment.
you are right, removed
| /** A line of output, forwarded as the tool that produced it phrased it. */ | ||
| | { kind: "output"; message: string }; | ||
|
|
||
| export type DeployProjectOptions = { |
There was a problem hiding this comment.
since all of these options are marked as required does input make more sense? I think "options" usually implies "optional".
There was a problem hiding this comment.
yes, I renamed this to DeployProjectInput, matching CreateProjectInput/ResolveProjectInput.
| @@ -158,8 +171,137 @@ export class FsProjectManager implements ProjectManager { | |||
| // The generated package.json defines `cdk` as "npm run build && cdk", so this | |||
There was a problem hiding this comment.
is this comment overly verbose? It feels a little distracting to me.
There was a problem hiding this comment.
haha yes I am editing all comments now
| /** Skip bootstrapping the target environment before deploying. */ | ||
| skipBootstrap: boolean; | ||
| /** Name of the aws-targets.json entry to deploy. */ | ||
| target: string; |
There was a problem hiding this comment.
is there a use case for deploying multiple targets?
There was a problem hiding this comment.
i dont see any really. --target is required and resolved before anything is built. so far, one deploy ships one target.
we can add a --all flag or the ability to deploy multiple ones in one go.
There was a problem hiding this comment.
planning to add --target DEFAULT as default in a follow up PR when I do the credential preflight threading. it is not in the scope for this PR
|
|
||
| // Drives a CDK operation, surfacing the toolkit's messages as project events and | ||
| // logging them. Anything below info stays in the debug log rather than on screen. | ||
| private async *streamCdk( |
There was a problem hiding this comment.
would this make sense on the cdk runner directly?
| if (!existsSync(path)) return []; | ||
| try { | ||
| return await this.json.read(path, AwsTargetsSchema); | ||
| } catch (cause) { |
There was a problem hiding this comment.
Would it be simpler to improve the error message on json.read instead of rewrapping here?
There was a problem hiding this comment.
DeserializationError already names the path, so the message isn't the issue. the rewrap adds the TARGETS_EXAMPLE (schema-specific, can't live in json.read) and the error source: DeserializationError is a plain Error, so fromError marks it INTERNAL and a user's typo counts as a CLI bug. ProjectStateError is USER.
that said, DeserializationError should probably be USER-sourced for every caller. what do you think? if so, I can raise a follow-up PR but it would affect everything so I don't wanna lump it in this already massive one :)
| * with the target's name, so deploy asks the assembly which stack to ship rather | ||
| * than deriving the name itself and hoping the two agree. | ||
| */ | ||
| export async function stackForTarget( |
There was a problem hiding this comment.
shouldn't the synth output the stack name? Is there a reason we need to re-derive it here?
There was a problem hiding this comment.
nothing is re-derived. stackForTarget reads synth's manifest.json and selects the stack whose agentcore:target-name tag matches, precisely so the CLI never reproduces the app's naming convention
| // whatever stale assembly it found there while reporting success. | ||
| yield { kind: "step", message: "Synthesizing CloudFormation templates" }; | ||
| await this.run( | ||
| ["npm", "run", "cdk", "--", "synth", "--quiet", "--output", this.assemblyPath(project)], |
There was a problem hiding this comment.
is there a way we can move more of the CDK specific logic into a separate class/module (maybe the CDK runner that already exists)?
I feel like the project manager is going to grow a lot as we add more project functionality, and I think it makes sense as a orchestration layer over the project functionality that shouldn't be concerned with implementations. For example, I feel like the scaffolding code that @tejaskash did is a nice example of separation of concerns we could try to follow.
There was a problem hiding this comment.
agree, I will move this to a backends/cdk.ts file
d9fd0b5 to
5e56870
Compare
The CDK toolkit narrates a deploy in hundreds of lines, which buried the steps deploy words itself. Those messages now go only to the shared file logger at the standard location, each keeping the toolkit's own severity, so a failure reads as an error in the log rather than as one debug line among thousands. What a deploy shows is its own steps, and it names where the detail is: Synthesizing CloudFormation templates Bootstrapping aws://111122223333/us-east-1 Deploying AgentCore-MyAgent-default Deployed project 'MyAgent' Detailed logs: ~/.agentcore/logs/output-2026-08-17.log The location is printed however the deploy ended, since a failed one is when the log matters most. It is shortened to `~` rather than made relative to the working directory: the log lives under the user's home, so a relative path from a project is a run of `../` that stops being correct as soon as they cd. src/logging/location.tsx owns the path, so the logger the entrypoint builds and the location deploy prints cannot disagree, and its test pins the predicted file name against the one a real rotating logger writes. With nothing forwarding the toolkit's output, ProjectEvent loses its `output` variant.
Review found the comments in the assembly reader and the CDK adapter longer than what they explain. Each keeps the fact a reader cannot get from the code -- why stack selection goes through the target tag, why a failure surfaces only after the output explaining it -- and drops the restatement of the code around it.
Terraform and no-IaC projects are coming, so the manager can no longer be the place that knows how a project is built. src/core/project/backends holds the seam: ProjectBackend is build and deploy as performed by the tool that owns a project's artifacts, CdkBackend is today's only implementation, and agentcore.json's `managedBy` selects one. Nothing above the seam names a tool -- FsProjectManager reads aws-targets.json, resolves the requested target, and hands it to the backend -- so a second backend is a new file and an entry in the dispatch table rather than a change to the manager or to any handler. A project declaring a backend this build has none for is reported as a configuration error naming the value, since `managedBy` is data a user can edit. The CDK tests move with the code. backends/cdk.test.ts covers synth, bootstrap and deploy ordering, the invariant that synth writes the assembly the toolkit is handed, and how the toolkit's messages reach the log; its project fixture is the directory tree the backend reads rather than a scaffolded project, so nothing chdirs. manager.test.ts keeps what the manager itself does -- target selection, and the errors an unusable aws-targets.json produces -- against a backend that records what it is handed instead of building anything. --skip-bootstrap goes with it. Bootstrap is idempotent and takes seconds on an environment that is already current, so a deploy runs it every time rather than asking the user to decide; the flag existed only in this branch, with nothing depending on it.
A project's log held every command's output in one file per day, so reading a failed deploy meant reading around every create, list and invoke that shared the day with it. Each command now writes into a directory of its own: agentcore/.cli/logs/deploy/deploy-2026-08-17.log Rotation is unchanged, so a command's directory holds one file per day, capped at ten. The name comes from argv, because the logger has to exist before there is a router to ask what is running: it is the last segment of the leading run of non-flag tokens, so `--region us-east-1 project deploy` and `project deploy --target prod` both name deploy, and a run with no command of its own -- the TUI -- is filed under the CLI. Only that last segment is used, which is why finding it needs no flag definitions. logFilePrefix stays the single place the path is built, so the file the entrypoint opens and the one deploy prints cannot disagree.
Upstream reshaped Project into { name, rootPath, spec }, so the manager now
reads project.spec.managedBy and ProjectEvent yields carry kind: "step". The
project dev command upstream wired in stays; its deploy registration is
dropped, since deploy is registered with withProject further down.
| /** | ||
| * Mirrors AwsDeploymentTarget in @aws/agentcore-cdk: each entry names the account and | ||
| * region the CDK app synthesizes a stack for. `agentcore project create` scaffolds an | ||
| * empty list, so a project has no targets until the user fills them in. | ||
| */ |
There was a problem hiding this comment.
follow up PR would add the aws credential pre-flight that we do currently in the CLI, if needed
…yped The log rotated daily, so every run of a command on the same day appended to one file and reading a deploy's log meant finding where that deploy started. main writes one timestamped file per run; this does the same, dropping winston-daily-rotate-file with the rotation it configured. The file also no longer takes its directory from the command line. The command was guessed by parsing argv, which a positional argument answers as readily as a subcommand: `config telemetry.enabled false` filed its log under `false`, and `config endpoint ../../../../../../tmp/pwn` under whatever that resolved to. A run's log is named for the CLI and the time it started, so nothing a user types decides where it is written.
Bun appends .exe when compiling for a Windows target whatever outfile it is given, so the build recorded a path that was never written: reading the executable back to report its size failed with ENOENT on the extensionless name, and the Windows build never got as far as uploading anything. The name now carries the extension, matching the file the build workflow smoke tests.
The toolkit returns a typed result naming every output of every stack it deployed — the runtime ARN, the gateway URL, the ids a caller needs — and it was thrown away: the toolkit's own narration went to the log file, so a deploy told the user it had deployed and nothing about what it had deployed. The outputs now travel back as the return value of the generator that streams the operation's messages, so progress stays progress and the result stays the result, and reach deploy as an `outputs` event alongside the steps it already reports. deploy prints them under the result, sorted, and prints nothing extra for a stack that declares none.
The log moved into whichever project the command was run in, so a user chasing a failure had to know which project directory to look in, and the same CLI wrote its logs to as many places as it had projects. It writes to ~/.agentcore/logs again, as it did before, and still prints the path it actually wrote. One file per run is kept: the file is named for the run, not the project.
… file One file per run bounds a run's log and nothing else: the rotating file it replaced kept ten files of five megabytes, and dropping it left a directory that only ever grew, a file per invocation, forever. Each run now deletes the logs it has made redundant before writing its own, keeping the fifty most recent runs and at most fifty megabytes of them -- the ceiling the rotating file enforced. Only the two names the CLI writes are considered, so the daily files an earlier version left behind are cleared too and anything else in the directory is left where it was put. A run never deletes the file it is still writing, and housekeeping that fails is housekeeping that did nothing rather than a command that failed.
One file per run, pruned at startup, replaced a transport that rotated daily and kept 5 MB × 10. Two mechanisms for bounding the same directory is one more than it needs, and the rotating one enforces a per-file size limit at write time rather than a directory-wide one a run later. So the transport, its dependency and `~/.agentcore/logs/output-<date>.log` are back as they were, and the run-scoped naming and the pruning that existed only to bound it are gone. What stays is `location.tsx` naming the file, since a command that prints where its log is has to name the file the transport opened: the prefix the transport is handed is not a path a user can open, and spelling the file in two places is how the printed path and the real one drift.
The log held the toolkit's account of a deploy and nothing of the deploy itself, so the hundreds of lines under a step gave no sign of which step they belonged to, and the outputs a deploy ended with were only ever on screen. Deploy now writes its steps and its result through the same logger the toolkit's narration goes through, with the outputs as bindings so they are data rather than the lines printed for them.
A deploy bootstrapped every time, and the parameters it bootstraps with override the stack's existing ones, so a deploy into an environment somebody had already bootstrapped without a customer-managed key rewrote the CDKToolkit stack the whole account shares. It also required CloudFormation write access on every deploy, which a deploy-only role does not have. So the bootstrap version is read first and bootstrap runs only when the stack is missing, unreadable or older than we deploy against — the same check the published CLI makes. The step is no longer printed when it does not happen.
…account early Three small ones from review: - The toolkit is EXTERNAL to the bundle, so the version a user resolves can report a log level this build has no mapping for. That indexed into undefined and took the whole deploy down over a log line; it now lands at info. - A deployment target's account becomes `aws://<account>/<region>`, so a typo used to surface minutes later from inside the toolkit. The schema requires 12 digits and names the field instead. - TestCoreClient handed its json adapter to the project manager but not to the backend it builds, so a backend's reads went to real disk while everything else read the test's fixtures.
The probe read the version output and nothing else, so a CDKToolkit that was rolled back or mid-delete reported version 32 and counted as ready: the deploy skipped the bootstrap that would have repaired it and failed later against roles and a bucket that were not there. It now requires the stack to be in one of the states the published CLI's own check accepts, and the parsing of the DescribeStacks response is a pure function with tests of its own rather than the one AWS-touching seam nothing covered. Also: the deploy logs what the probe decided, since it decides whether the step below runs at all; the version floor now says where the published CLI sets it, so the number is checkable; and two comments that overclaimed are corrected — a run crossing midnight rotates into the next day's file, and a rejected account fails the file rather than naming the field.
This PR adds the project deploy command. it resolves the deployment target, synthesizes, bootstraps that target's environment, then deploys its stack. Synth reuses build's code path, so build's dependency check runs before anything touches AWS.
stack environments come from
agentcore/aws-targets.json, which create() scaffolds empty. An empty list is an error naming the file to fill in, rather than an account resolved from the active credentials, deploy shouldn't guess where the user's infra belongs. It's checked before synth, so an empty list fails without running anything.one deploy ships one target.
--targetselects it and defaults to default, so a project with a staging and a prod target can't reach prod by accident. The target is resolved before synth, so a misspelled --target costs no build and the error lists the configured names. which stack belongs to the target comes from the synthesized manifest, matched on theagentcore:target-nametag the generated CDK app already writes, so the CLI never has to reproduce that app's stack-naming convention.bootstrap runs only where it's needed: a deploy reads
CDKToolkit'sBootstrapVersionfirst and bootstraps only when the stack is missing, unreadable, or older than version 30, the same minimum the published CLI requires. It still bootstraps withcreateCustomerMasterKey: trueto match that CLI, so a first deploy provisions a KMS key for the staging bucket. probing first is what stops a later deploy from rewriting aCDKToolkitsomebody else bootstrapped without one.Not in this PR:
This is the synth -> bootstrap -> deploy spine. Everything else the existing deploy does is deliberately left out for now:
--allor multi-target deploy. Follow-up.